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