Re: [PATCH] i386: Do not modify existing RTL (PR66412)

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

On Fri, Jun 26, 2015 at 09:42:06AM +0200, Uros Bizjak wrote:
> Hello!
> 
> > > > [ I see i386 also does PUT_CODE in a few more splitters, hrm. ]
> > >
> > > I think the splitters need to be fixed but I also think that
> > > shallow_copy_rtx should
> > > be enough, no?  combine will not actually end up generating more references
> > > to the original RTX so we won't end up with invalid RTX sharing (the same logic
> > > is why the splitters didn't care).
> >
> > That probably is enough; I haven't thought about it too much, all other
> > splitters doing a copy do a deep copy.  I don't think it is worth being
> > too tricky here, those splitters do not match often at all.
> >
> > > It would be cleaner to write the splitters in a way that doesnt copy the RTX
> > > but creates a new RTX with proper code/mode and operands (but that's more
> > > work on your side - maybe Uros wants to help here).
> >
> > I think people use "copy_rtx; PUT_*" because it is less typing work ;-)
> > If that is the only thing you change about the rtx, it even is easier
> > to understand than creating a new one.
> 
> Attached patch implements both suggestions. It uses shallow_copy_rtx
> before all PUT_* calls in the splitters, to avoid nasty surprises like
> this one in the future. shallow_ropy_rtx copies just one level of RTX,
> and this is all what we need. Also, the patch changes existing uses of
> copy_rtx to shallow_copy_rtx.

Thanks for doing the legwork Uros!  Much appreciated.

Andreas, s390 does a bit of this too (in vector.md).  I cannot really
test for that at all, can you take care of it?


Segher