Re: PATCH: PR target/40470: unable to find a register to spill in class âSSE_FIRST_REG â

[Graham Stott <graham dot stott at btinternet dot com>]

Jeff,

cant_combine_insn_p looks to be where the combind should get rejected.

It uses CLASS_LIKELY_SPILLED (REGNO_REG_CLASS (REGNO (...)) to reject
the combine if it finds a HARD reg which is likely to be spilled.

Now on the x86 XMM0 is in MMX_REGS class which is NOT a likely spilled class hence it doen't get rejected.

Adding MMX_REGS to the set of likely spilled classes on x86 should do the
trick.


Cheers
Graham

--- On Thu, 18/6/09, Jeff Law <law@redhat.com> wrote:

> From: Jeff Law <law@redhat.com>
> Subject: Re: PATCH: PR target/40470: unable to find a register to spill in class âSSE_FIRST_REGâ
> To: "H.J. Lu" <hjl.tools@gmail.com>
> Cc: "Vladimir Makarov" <vmakarov@redhat.com>, "Steven Bosscher" <stevenb.gcc@gmail.com>, "Richard Guenther" <richard.guenther@gmail.com>, gcc-patches@gcc.gnu.org, ubizjak@gmail.com
> Date: Thursday, 18 June, 2009, 6:49 PM
> H.J. Lu wrote:
> > On Thu, Jun 18, 2009 at 10:37 AM, Jeff Law<law@redhat.com>
> wrote:
> >ÂÂÂ
> >> H.J. Lu wrote:
> >>Â ÂÂÂ
> >>> On Wed, Jun 17, 2009 at 11:46 AM, Vladimir
> Makarov<vmakarov@redhat.com>
> >>> wrote:
> >>>
> >>>Â Â ÂÂÂ
> >>>> Jeff Law wrote:
> >>>>
> >>>>Â Â Â ÂÂÂ
> >>>>> Vladimir Makarov wrote:
> >>>>>
> >>>>>Â Â Â Â
> ÂÂÂ
> >>>>>> IRA does not generate spill code
> for insn constraints. Reload does it.
> >>>>>>Â To fix it in reload (spill
> and restore hard registers) is not a
> >>>>>> trivial
> >>>>>> task and potentially distracting
> compiler stability for many targets.
> >>>>>>Â So
> >>>>>> I'd rather avoid it even if it
> could solve other passes problems like
> >>>>>> 1st
> >>>>>> insn scheduling for x86/x86_64.
> >>>>>>
> >>>>>> The originally problem occurs in
> the combiner (it propagates xmm0 usage
> >>>>>> right after a call through the
> insn in question). So imho I think the
> >>>>>> best
> >>>>>> way is to fix in the combiner.
> >>>>>>
> >>>>>>Â Â Â Â Â
> ÂÂÂ
> >>>>> Is xmm0 the return value for the
> call? ie, is it the SET_DEST of the
> >>>>> call?
> >>>>>
> >>>>>
> >>>>>
> >>>>>Â Â Â Â
> ÂÂÂ
> >>>> I looked at this problem. It looks
> like
> >>>>
> >>>> xmm0=call
> >>>> p666= xmm0
> >>>> ...
> >>>> insn in the question (spill failure
> because it needs xmm0)
> >>>> ...
> >>>> = p666, dead p666
> >>>>
> >>>> The combiner removes p666=xmm0 and uses
> xmm0 instead of p666 in the last
> >>>> insn.
> >>>>
> >>>>
> >>>>Â Â Â ÂÂÂ
> >>> xmm0 is used for function return:
> >>>
> >>> (call_insn 19 18 20 4 x.i:31 (set (reg:V2DI 21
> xmm0)
> >>>Â Â Â Â (call (mem:QI
> (symbol_ref:DI ("load") [flags 0x41]
> >>> <function_decl 0x7ffff310c800 load>) [0
> S1 A8])
> >>>Â Â Â Â Â Â
> (const_int 0 [0x0]))) 908 {*call_value_0_rex64}
> >>> (expr_list:REG_DEAD (reg:DI 5 di)
> >>>Â Â Â Â (nil))
> >>>Â Â (expr_list:REG_DEP_TRUE (use
> (reg:DI 5 di))
> >>>Â Â Â Â (nil)))
> >>>
> >>> xmm0 is also a fixed register for some SSE4
> insns. Since
> >>> we don't assign xmm0 early for those insns,
> how can
> >>> combine do anything for them? I think the
> proper fix
> >>> is in RA/reload.
> >>>
> >>>Â Â ÂÂÂ
> >> No. The right place to fix this is in
> combine until such time as we have a
> >> new reload pass. ÂÂÂCombine
> has had code for decades to not extend the
> >> lifetime of a hard return value to avoid this
> exact set of problems. It's
> >> clearly not working and that's what needs to be
> fixed.
> >>
> >>Â ÂÂÂ
> >
> > The return value in xmm0 is used in the following few
> insns which also
> > require the fixed xmm0 for a different operand. How
> does combine
> > know it shouldn't extend the lifetime of a hard return
> value when it is
> > used immediately in the next few insns?
> >
> >ÂÂÂ
> The whole point of copying xmm0 to p666 is to free
> xmm0Â so that it can 
> be used by other insns.
> 
> Combine's removal of p666 = xmm0 is wrong as is changing
> the last insn 
> above to use xmm0 instead of p666.
> 
> 
> 
> jeff
> 
>