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

[Segher Boessenkool]

On Thu, Jun 25, 2015 at 11:59:28AM +0200, Richard Biener wrote:
> On Thu, Jun 25, 2015 at 7:11 AM, Segher Boessenkool
> <segher@kernel.crashing.org> wrote:
> > On Wed, Jun 24, 2015 at 09:40:28PM -0600, Jeff Law wrote:
> >> On 06/24/2015 05:29 PM, Segher Boessenkool wrote:
> >> >A few define_split's in the i386 backend modify RTL in place.  This does
> >> >not work.  This patch fixes all cases that do PUT_MODE on existing RTL.
> >
> >> >     * config/i386/i386.md (various splitters): Use copy_rtx before
> >> >     doing PUT_MODE on operands.
> >> Are the copies really needed?  If we're slamming a mode into an
> >> ix86_comparison_operator, we should be safe since those can't be shared.
> >>  Copying is just wasteful.
> >
> > combine still holds pointers to the old rtx, which is what is causing
> > the problem in the PR (it does always unshare things in the end, but
> > it does not make copies while it's working).  Either those few splitters
> > need to do the copy (and some already do), or combine has to do the copy
> > always, which would be more wasteful.
> >
> > It has always been this way as far as I see?  Am I missing something?
> >
> > [ 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.


Segher