On Sat, Mar 20, 2010 at 6:24 AM, Jim Wilson <wil...@codesourcery.com> wrote: > On Fri, 2010-03-19 at 22:06 +0800, fanqifei wrote: >> 1. I add movsi expander in which function foor_expand_move is used to >> force the operands[1] to reg and emit the move insn. >> Another change is that in the define of insn “*mov_mode_insn" in which >> a condition is added to prevent a store const to mem RTL insn from >> being accepted. >> Are these changes necessary? > > Yes, this looks correct and necessary. > >> 2. Is is correct to use emit_move_insn in foor_expand_move? >> in mips.md, the function mips_emit_move called both emit_move_insn and >> emit_move_insn_1. But I don’t quite understand the comment above the >> function. > > This looks like the kind of thing you don't need to understand now. > Just call emit_move_insn, and worry about bizarre details like this > later. It isn't obvious to me why it is there either. > > Before reload, you can create new pseudo-regs at any time if you need to > load something into a register. After reload you can't. > emit_move_insn_1 assumes its operands are valid. emit_move_insn checks > to see if operands are valid and if not tries to fix them. Calling > emit_move_insn after reload will fail if you have an invalid operand > that needs to be loaded into a new pseudo. Calling emit_move_insn_1 > with invalid operands will fail a different way. It looks like the mips > port is trying to do something very tricky and subtle here. If you want > to understand it, you probably have to find the patch that added it. Or > find a testcase where it makes a difference. > >> 3. My understanding of the internal flow about the issue is: >> The named insn “movsi” is used to generate RTL insn list from parse >> tree. The insn pattern “set mem, const” is expanded by function >> foor_expand_move(). For other forms of “set” insns, the template given >> in the pattern is inserted. Then the insn "*mov_mode_insn" is used to >> generate assembler code. In the generation, the condition of >> mov_mode_insn is checked. >> I am not fully confident the understanding is correct. > > That seems correct. movsi is used for generating RTL. mov_mode_insn is > used for matching RTL. > >> (define_insn "*mov_mode_insn" >> [(set >> (match_operand:BWD 0 "nonimmediate_operand" "=r,m,r,r,r,r,r,r,x,r") >> (match_operand:BWD 1 "foor_move_source_operand" >> "Z,r,L,I,Q,P,ni,x,r,r"))] >> "(!( >> (memory_operand(operands[0], SImode) && >> (foor_const_operand_f(operands[1]))) >> ||(memory_operand(operands[0], HImode) && >> (foor_const_operand_f(operands[1]))) >> ||(memory_operand(operands[0], QImode) && >> (foor_const_operand_f(operands[1]))) > > BWD is presumably a mode macro. You can use <BWD>mode to get the enum > mode name instead of having 3 copies of the test. Checking to reject > mem&&const is equivalent to checking to accept reg||reg. The latter > check is the conventional one and will be faster, as register_operand > does less work than memory_operand, and short-cut evaluation means we > only need one register_operand call in the common case. This assumes > that 'x' is some kind of register which seems likely. > >> predicates.md: >> (define_predicate "foor_const_operand" >> (match_test "foor_const_operand_f(op)")) > > You don't need the foor_const_operand function. You can just do > (match_code "const_int") > > Jim > > > I changed the condition in "*mov_insn_mode" to below: "( (register_operand(operands[0], SImode) || register_operand(operands[1],SImode)) ||(register_operand(operands[0], HImode) || register_operand(operands[1],HImode)) ||(register_operand(operands[0], QImode) || register_operand(operands[1],QImode)) )" Then there is an error during gcc build: ../.././gcc/tmplibgcc_fp_bit.c: In function '_fpadd_parts': ../.././gcc/tmplibgcc_fp_bit.c:740: error: unrecognizable insn: (insn 41 40 42 12 ../.././gcc/tmplibgcc_fp_bit.c:637 (set (mem/s:SI (reg/v/f:SI 43 [ tmp ]) [2 S4 A32]) (mem/s:SI (reg/v/f:SI 41 [ a ]) [2 S4 A32])) -1 (nil)) ../.././gcc/tmplibgcc_fp_bit.c:740: internal compiler error: in extract_insn, at recog.c:1990 Please submit a full bug report,
Seems that the pattern "set mem,mem" is not recognized. But how was it recognized when I was using mem&&const? The constraints don't contain "m"&&"m". I don't know what extra information should I provide now. Thanks very much! -- -Qifei Fan http://freshtime.org