Re: [PATCH][combine][RFC] Don't transform sign and zero extends inside mults

[Jeff Law <law at redhat dot com>]

On 11/06/2015 07:19 AM, Kyrill Tkachov wrote:
> > > I think we should also add:
> > >        && REG_P (XEXP (XEXP (x, 0), 0))
> > >        && REG_P (XEXP (XEXP (x, 1), 0))
I tend to agree.

> > Yep, see new patch.  The "from == to" condition is for when subst is
> > called
> > just to simplify some code (normally with pc_rtx, pc_rtx).
>
> Indeed, this looks better but it still needs the REG_P checks for the inner
> operands of the extends to not screw up the arm case.
> Here's my proposed extension. I've also modified the testcase slightly
> to not use
> an unitialised variable. It still demonstrates the issue we're trying to
> solve.
>
> Bootstrapped and tested on arm and aarch64.
> I'll let you put it through it's paces on your setup :)
>
> P.S. Do we want to restrict this to targets that have a widening mul
> optab like I did in the original patch?
I don't think it's necessary or desirable.  Consider the case where the 
target has a limited widening multiply insn -- we may not want to expose 
it to the RTL expanders due to some limitation, but we still want the 
combiner to be able to create it under the right circumstances.

> Thanks,
> Kyrill
>
> 2015-11-06  Segher Boessenkool  <segher@kernel.crashing.org>
>              Kyrylo Tkachov  <kyrylo.tkachov@arm.com>
>
>      * combine.c (subst): Don't substitute or simplify when
>      handling register-wise widening multiply.
>      (force_to_mode): Likewise.
>
> 2015-11-06  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>
>
>      * gcc.target/aarch64/umaddl_combine_1.c: New test.
I'll let Segher give the final yes/no on this, but it generally looks 
good to me.

jeff