Re: [PATCH v2, rtl-optimization]: Fix PR46829, ICE in spill_failure with -fschedule-insns [was: Fixing instability of -fschedule-insns for x86]

[Uros Bizjak <ubizjak at gmail dot com>]

On Tue, Aug 21, 2012 at 1:34 AM, Oleg Endo <oleg.endo@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 <ubizjak@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.