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

[Uros Bizjak <ubizjak at gmail dot com>]

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.

2015-06-26  Uros Bizjak  <ubizjak@gmail.com>
        Segher Boessenkool  <segher@kernel.crashing.org>

    * config/i386/i386.md (various splitters): Use shallow_copy_rtx
    before doing PUT_MODE or PUT_CODE on operands to avoid
    in-place RTX modification.

testsuite/ChangeLog:

2015-06-26  Uros Bizjak  <ubizjak@gmail.com>

    * gcc.target/i386/pr66412.c: New test.

Patch was bootstrapped and regresison tested on x85_64-linux-gnu
{,-m32}. Patch was committed to mainline SVN and will be backported to
all release branches (once they open).

Uros.

Attachment: p.diff.txt
Description: Text document