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
