On Tue, Aug 21, 2012 at 1:34 AM, Oleg Endo <oleg.e...@t-online.de> wrote: > On Sat, 2012-08-18 at 10:23 +0200, Uros Bizjak wrote: >> On Sat, Aug 18, 2012 at 10:14 AM, Uros Bizjak <ubiz...@gmail.com> wrote: >> >> > After discussion with Oleg, it looks that it is enough to prevent >> > wrong registers in the output of the (multi-output) insn pattern. As >> > far as inputs are concerned, combine already handles limited reload >> > classes in the right way. The problem with x86 is, that reload tried >> > to fix output operand of the multi-output ins, where scheduler already >> > moved load of ax register before this insn. >> > >> > Version 2 of the patch now handles only output operands. Also, >> > handling of empty constraints was fixed. >> >> ... but here we fail testcase from PR46843... so we HAVE to check >> input operands. > > Hm, I'm curious ... how would that work for patterns such as > > (define_insn "*addc" > [(set (match_operand:SI 0 "arith_reg_dest" "=r") > (plus:SI (plus:SI > (match_operand:SI 1 "arith_reg_operand" "%0") > (match_operand:SI 2 "arith_reg_or_0_operand" "r")) > (match_operand:SI 3 "t_reg_operand" ""))) > (clobber (reg:SI T_REG))] > "TARGET_SH1" > "addc %2,%0" > [(set_attr "type" "arith")]) > > ... where the predicate "arith_reg_or_0_operand" allows either > "const_int 0" or an "arith_reg_operand", but the constraint "r" tells > reload to load the constant into a register for this insn. > Probably those kind of patterns that rely on this behavior would need to > be rewritten as insn_and_split to do the constant loading 'manually'?
I think that we have to introduce a target hook that would "approve" the combined insn. This way, every target can analyse the combined insn and handle it in the most appropriate way. Uros.
