rs6000: Fix up flag_shrink_wrap handling in presence of -mrop-protect [PR101324]

Peter Bergner bergner@linux.ibm.com
Thu Nov 11 22:03:43 GMT 2021


Sorry for taking so long to get back to this.

On 10/29/21 4:45 PM, Segher Boessenkool wrote:
> On Wed, Oct 27, 2021 at 10:17:39PM -0500, Peter Bergner wrote:
>> +/* Ensure hashst comes after mflr and hashchk comes after ld 0,16(1).  */
>> +/* { dg-final { scan-assembler "mflr 0.*hashst 0," } } */
>> +/* { dg-final { scan-assembler "ld 0,16\\\(1\\\).*hashchk 0," } } */
> 
> First: don't use double quotes, or you get double backslashes (or more)
> as well.  Use curlies instead:
> 
> /* { dg-final { scan-assembler {ld 0,16\(1\).*hashchk 0,} } } */
> 
> But, more importantly, "." by default matches anything, newlines as
> well.  You probably do not want that here, because your RE as written
> can match an "ld" in one function and a "hashchk" many functions later,
> many million lines later.
> 
> You can for example do
> /* { dg-final { scan-assembler {(?p)ld 0,.*\n.*\mhashchk 0,} } } */

I had to change your suggestion to the following, which works:

/* { dg-final { scan-assembler {(?p)\mmflr 0.*\n.*\n.*\mhashst 0,} } } */
/* { dg-final { scan-assembler {(?p)ld 0,.*\n.*\n.*\n.*\mhashchk 0,} } } */


Meaning from the current code generation, the hashst is 2 insns after
the mflr insn and the hashck is 3 insns after the ld 0,16(1) insn.
But is this fragile?  Are we sure we won't schedule the hashst and
hashchk to some other location breaking the test case?



> (?p) is "partial newline-sensitive matching": it makes "." not match
> newlines.  This is often what you want.  This RE also makes sure that
> "hashchk" is the full mnemonic (not the tail of one), and that it is on
> the line after that "ld".

Well, this test case only has one function and is very small, so there
should only be one "mflr 0" and one "ld 0,16(1)".  Given the small size,
I assumed if the hashst and hashchk were after the mflr/ld 0,16(1), then
it's in the correct order and "fixed".  But maybe that's just as fragile
as assuming how many insns proceed the hashst/hashchk?


If you prefer the updated change to your suggestion, let me know and I'll
commit it that way. 


> I'll note the test case uses the "new" rop_ok effective-target function which
> I submitted as a separate patch.

This test case uses the new rop_ok effective-target which I said I submitted
as a separate patch, but I can't seem to find the submission anywhere and it's
not even in the archive.  Did I forget to submit it?  Probably.  :-(
Let me do that now, as this relies on that patch.


Peter



More information about the Gcc-patches mailing list