Re: [PATCH] rs6000: Add "cannot_copy" attribute, use it (PR67788, PR67789)

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

On Thu, Oct 01, 2015 at 12:14:44PM +0200, Richard Biener wrote:
> On Thu, Oct 1, 2015 at 8:08 AM, Segher Boessenkool
> <segher@kernel.crashing.org> wrote:
> > After the shrink-wrapping patches the prologue will often be pushed
> > "deeper" into the function, which in turn means the software trace cache
> > pass will more often want to duplicate the basic block containing the
> > prologue.  This caused failures for 32-bit SVR4 with -msecure-plt PIC.
> >
> > This configuration uses the load_toc_v4_PIC_1 instruction, which creates
> > assembler labels without using the normal machinery for that.  If now
> > the compiler decides to duplicate the insn, it will emit the same label
> > twice.  Boom.
> >
> > It isn't so easy to fix this to use labels the compiler knows about (let
> > alone test that properly).  Instead, this patch wires up a "cannot_copy"
> > attribute to be used by TARGET_CANNOT_COPY_P, and sets that attribute on
> > these insns we do not want copied.
> >
> > Bootstrapped and tested on powerpc64-linux, with the usual configurations
> > (-m32,-m32/-mpowerpc64,-m64,-m64/-mlra); new testcase fails before, works
> > after (on 32-bit).
> >
> > Is this okay for mainline?
> 
> Isn't that quite expensive?

Not really?  recog_memoized isn't so bad.  I cannot measure a difference.

> So even if not "easy", can you try?

I did, and after half a day had a big mess and lots of things failing,
no idea where this was headed, and in the meantime bootstrap still fails
(on affected targets).

Other targets use cannot_copy_p in similar situations.

> Do we have other ports with local labels in define_insns?

I of course tried to find such, but didn't.  Oh, you're asking for any
insn that does anything whatsoever, but with a label.  That's the
"half a day" above.

It might matter that these insns are created after reload.  Or I somehow
need to force the BB to be split, that seems to have been the problem;
and splitting the prologue will be FUN.

> I see some in darwin.md as well which your patch doesn't handle btw.,

Oh argh forgot to grep outside of rs6000.md.  Will fix.

> otherwise suspicious %0: also appears (only) in h8300.md.

I think it is part of the syntax for that insn?   jsr ...:8

> arc.md also has
> a suspicious case in its doloop_end_i pattern.

That one is in an assembler comment  :-)


Segher