Re: [PATCH 2/5] combine: handle I2 a parallel of two SETs

[Segher Boessenkool <segher at kernel dot crashing dot org>]

On Tue, Nov 25, 2014 at 11:35:14AM -0700, Jeff Law wrote:
> On 11/14/14 12:19, Segher Boessenkool wrote:
> >If I2 is a PARALLEL of two SETs, split it into two instructions, I1
> >and I2.  If there already was an I1, rename it to I0.  If there
> >already was an I0, don't do anything.
> >
> >This surprisingly simple patch is enough to let combine handle such
> >PARALLELs properly.
> It's clever.

It does a very similar thing as the code right before it.

> So you're virtually serializing the PARALLEL to make combine happy if 
> I'm reading this correctly.

That is correct.

> THe first thing I worry about is preserving the semantics of a PARALLEL. 
>  Specifically that all the inputs are evaluated, then all the side 
> effects happen.  So I think one of the checks you need is that the 
> destinations of the SETs are not used as source operands in the SETs.

As you say below, Bernd also noticed this; I haven't found the time to
make a new patch yet.  "Soon".

> The second thing I worry about handling of match_dup operands.  But 
> presumably all the resulting insns must match in one way or another or 
> the whole thing gets reset to its prior state.  So I suspect those are 
> OK as well.
> 
> Related, I was worried about RTL structure sharing, but in the end I 
> think those are a non-concern for the same basic reasons as match_dups 
> aren't a real worry.

combine make a copy of any RTL before it modifies it.

> >+  /* If I2 is a PARALLEL of two SETs of REGs (and perhaps some CLOBBERs),
> >+     make those two SETs separate I1 and I2 insns, and make an I0 that is
> >+     the original I1.  */
> >+  if (i0 == 0
> Test for NULL.

All similar code in combine tests for 0.  I'd have written "if (!i0)"
otherwise.

> And I think while convention has CLOBBERs at the end of insns, I don't 
> think that's a hard requirement.  So I think you need a stronger check 
> for elements 2 and beyond in the vector.

I'll check if 0,1 are SETs and all of 2..N-1 are CLOBBERs.  I was thinking
there could be nothing else but SETs and CLOBBERs (which combine already
does expect to always be in that order), but let's be less tricky.

> OK with the direction this is going, but I think another iteration is 
> going to be necessary.

Great, new patch coming up.


Segher