On Thu, Jun 25, 2015 at 7:11 AM, Segher Boessenkool <seg...@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). 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). Thanks, Richard. > > Segher
