diff mbox series

[rs6000] Use $ instead of . for PC

Message ID 43608c3f-c4ca-6fce-ea47-4cb2b6860ec5@linux.vnet.ibm.com
State New
Headers show
Series [rs6000] Use $ instead of . for PC | expand

Commit Message

Bill Schmidt Jan. 19, 2018, 8:58 p.m. UTC
Hi,

My recent patches to trunk and gcc-7-branch for avoiding speculation of
indirect branches has a flaw, pointed out by David.  Usage of "." to
represent the program counter is not portable across all POWER
assemblers, particularly not being accepted on AIX.  "$" is the 
universally accepted alternative.  So change the code and the test
cases to use $ instead of . for this purpose.

Regstrap is in progress on powerpc64-linux-gnu and powerpc64le-linux-gnu.
Assuming no issues are found, is this okay for trunk and backport to 7?

Thanks,
Bill


[gcc]

2018-01-19  Bill Schmidt  <wschmidt@linux.vnet.ibm.com>

	* config/rs6000/rs6000.md (*sibcall_nonlocal_sysv<mode>): Change
	assembly output from . to $.
	(*sibcall_value_nonlocal_sysv<mode>): Likewise.
	(indirect_jump<mode>_nospec): Likewise.
	(*tablejump<mode>_internal1_nospec): Likewise.

[gcc/testsuite]

2018-01-19  Bill Schmidt  <wschmidt@linux.vnet.ibm.com>

	* gcc.target/powerpc/safe-indirect-jump-2.c: Change expected
	assembly output from . to $.
	* gcc.target/powerpc/safe-indirect-jump-3.c: Likewise.
	* gcc.target/powerpc/safe-indirect-jump-8.c: Likewise.

Comments

Bill Schmidt Jan. 19, 2018, 9:04 p.m. UTC | #1
I see that David already proposed this same patch in PR83946.  Sorry, I've gotten behind on my email.

Two changes I need:  The scan-assembly should have \$ rather than $ in it, and I should add
PR83946 to the ChangeLog.

Sorry for the noise.

-- Bill

Bill Schmidt, Ph.D.
STSM, GCC Architect for Linux on POWER
Linux on Power Toolchain
IBM Linux Technology Center
wschmidt@linux.vnet.ibm.com

> On Jan 19, 2018, at 2:58 PM, Bill Schmidt <wschmidt@linux.vnet.ibm.com> wrote:
> 
> Hi,
> 
> My recent patches to trunk and gcc-7-branch for avoiding speculation of
> indirect branches has a flaw, pointed out by David.  Usage of "." to
> represent the program counter is not portable across all POWER
> assemblers, particularly not being accepted on AIX.  "$" is the 
> universally accepted alternative.  So change the code and the test
> cases to use $ instead of . for this purpose.
> 
> Regstrap is in progress on powerpc64-linux-gnu and powerpc64le-linux-gnu.
> Assuming no issues are found, is this okay for trunk and backport to 7?
> 
> Thanks,
> Bill
> 
> 
> [gcc]
> 
> 2018-01-19  Bill Schmidt  <wschmidt@linux.vnet.ibm.com>
> 
> 	* config/rs6000/rs6000.md (*sibcall_nonlocal_sysv<mode>): Change
> 	assembly output from . to $.
> 	(*sibcall_value_nonlocal_sysv<mode>): Likewise.
> 	(indirect_jump<mode>_nospec): Likewise.
> 	(*tablejump<mode>_internal1_nospec): Likewise.
> 
> [gcc/testsuite]
> 
> 2018-01-19  Bill Schmidt  <wschmidt@linux.vnet.ibm.com>
> 
> 	* gcc.target/powerpc/safe-indirect-jump-2.c: Change expected
> 	assembly output from . to $.
> 	* gcc.target/powerpc/safe-indirect-jump-3.c: Likewise.
> 	* gcc.target/powerpc/safe-indirect-jump-8.c: Likewise.
> 
> 
> Index: gcc/config/rs6000/rs6000.md
> ===================================================================
> --- gcc/config/rs6000/rs6000.md	(revision 256894)
> +++ gcc/config/rs6000/rs6000.md	(working copy)
> @@ -10987,7 +10987,7 @@
> 	return \"b%T0\";
>       else
> 	/* Can use CR0 since it is volatile across sibcalls.  */
> -	return \"crset eq\;beq%T0-\;b .\";
> +	return \"crset eq\;beq%T0-\;b $\";
>     }
>   else if (DEFAULT_ABI == ABI_V4 && flag_pic)
>     {
> @@ -11044,7 +11044,7 @@
> 	return \"b%T1\";
>       else
> 	/* Can use CR0 since it is volatile across sibcalls.  */
> -	return \"crset eq\;beq%T1-\;b .\";
> +	return \"crset eq\;beq%T1-\;b $\";
>     }
>   else if (DEFAULT_ABI == ABI_V4 && flag_pic)
>     {
> @@ -12566,7 +12566,7 @@
>   [(set (pc) (match_operand:P 0 "register_operand" "c,*l"))
>    (clobber (match_operand:CC 1 "cc_reg_operand" "=y,y"))]
>   "!rs6000_speculate_indirect_jumps"
> -  "crset %E1\;beq%T0- %1\;b ."
> +  "crset %E1\;beq%T0- %1\;b $"
>   [(set_attr "type" "jmpreg")
>    (set_attr "length" "12")])
> 
> @@ -12672,7 +12672,7 @@
>    (use (label_ref (match_operand 1)))
>    (clobber (match_operand:CC 2 "cc_reg_operand" "=y,y"))]
>   "!rs6000_speculate_indirect_jumps"
> -  "crset %E2\;beq%T0- %2\;b ."
> +  "crset %E2\;beq%T0- %2\;b $"
>   [(set_attr "type" "jmpreg")
>    (set_attr "length" "12")])
> 
> Index: gcc/testsuite/gcc.target/powerpc/safe-indirect-jump-2.c
> ===================================================================
> --- gcc/testsuite/gcc.target/powerpc/safe-indirect-jump-2.c	(revision 256894)
> +++ gcc/testsuite/gcc.target/powerpc/safe-indirect-jump-2.c	(working copy)
> @@ -30,4 +30,4 @@ int foo (int x)
> 
> /* { dg-final { scan-assembler "crset 30" } } */
> /* { dg-final { scan-assembler "beqctr- 7" } } */
> -/* { dg-final { scan-assembler "b ." } } */
> +/* { dg-final { scan-assembler "b $" } } */
> Index: gcc/testsuite/gcc.target/powerpc/safe-indirect-jump-3.c
> ===================================================================
> --- gcc/testsuite/gcc.target/powerpc/safe-indirect-jump-3.c	(revision 256894)
> +++ gcc/testsuite/gcc.target/powerpc/safe-indirect-jump-3.c	(working copy)
> @@ -49,4 +49,4 @@ int foo (int x)
> 
> /* { dg-final { scan-assembler "crset 30" } } */
> /* { dg-final { scan-assembler "beqctr- 7" } } */
> -/* { dg-final { scan-assembler "b ." } } */
> +/* { dg-final { scan-assembler "b $" } } */
> Index: gcc/testsuite/gcc.target/powerpc/safe-indirect-jump-8.c
> ===================================================================
> --- gcc/testsuite/gcc.target/powerpc/safe-indirect-jump-8.c	(revision 256894)
> +++ gcc/testsuite/gcc.target/powerpc/safe-indirect-jump-8.c	(working copy)
> @@ -12,4 +12,4 @@ int bar ()
> 
> /* { dg-final { scan-assembler "crset eq" } } */
> /* { dg-final { scan-assembler "beqctr-" } } */
> -/* { dg-final { scan-assembler "b ." } } */
> +/* { dg-final { scan-assembler "b $" } } */
> Index: gcc/config/rs6000/rs6000.md
> ===================================================================
> --- gcc/config/rs6000/rs6000.md	(revision 256894)
> +++ gcc/config/rs6000/rs6000.md	(working copy)
> @@ -10987,7 +10987,7 @@
> 	return \"b%T0\";
>       else
> 	/* Can use CR0 since it is volatile across sibcalls.  */
> -	return \"crset eq\;beq%T0-\;b .\";
> +	return \"crset eq\;beq%T0-\;b $\";
>     }
>   else if (DEFAULT_ABI == ABI_V4 && flag_pic)
>     {
> @@ -11044,7 +11044,7 @@
> 	return \"b%T1\";
>       else
> 	/* Can use CR0 since it is volatile across sibcalls.  */
> -	return \"crset eq\;beq%T1-\;b .\";
> +	return \"crset eq\;beq%T1-\;b $\";
>     }
>   else if (DEFAULT_ABI == ABI_V4 && flag_pic)
>     {
> @@ -12566,7 +12566,7 @@
>   [(set (pc) (match_operand:P 0 "register_operand" "c,*l"))
>    (clobber (match_operand:CC 1 "cc_reg_operand" "=y,y"))]
>   "!rs6000_speculate_indirect_jumps"
> -  "crset %E1\;beq%T0- %1\;b ."
> +  "crset %E1\;beq%T0- %1\;b $"
>   [(set_attr "type" "jmpreg")
>    (set_attr "length" "12")])
> 
> @@ -12672,7 +12672,7 @@
>    (use (label_ref (match_operand 1)))
>    (clobber (match_operand:CC 2 "cc_reg_operand" "=y,y"))]
>   "!rs6000_speculate_indirect_jumps"
> -  "crset %E2\;beq%T0- %2\;b ."
> +  "crset %E2\;beq%T0- %2\;b $"
>   [(set_attr "type" "jmpreg")
>    (set_attr "length" "12")])
> 
> Index: gcc/testsuite/gcc.target/powerpc/safe-indirect-jump-2.c
> ===================================================================
> --- gcc/testsuite/gcc.target/powerpc/safe-indirect-jump-2.c	(revision 256894)
> +++ gcc/testsuite/gcc.target/powerpc/safe-indirect-jump-2.c	(working copy)
> @@ -30,4 +30,4 @@ int foo (int x)
> 
> /* { dg-final { scan-assembler "crset 30" } } */
> /* { dg-final { scan-assembler "beqctr- 7" } } */
> -/* { dg-final { scan-assembler "b ." } } */
> +/* { dg-final { scan-assembler "b $" } } */
> Index: gcc/testsuite/gcc.target/powerpc/safe-indirect-jump-3.c
> ===================================================================
> --- gcc/testsuite/gcc.target/powerpc/safe-indirect-jump-3.c	(revision 256894)
> +++ gcc/testsuite/gcc.target/powerpc/safe-indirect-jump-3.c	(working copy)
> @@ -49,4 +49,4 @@ int foo (int x)
> 
> /* { dg-final { scan-assembler "crset 30" } } */
> /* { dg-final { scan-assembler "beqctr- 7" } } */
> -/* { dg-final { scan-assembler "b ." } } */
> +/* { dg-final { scan-assembler "b $" } } */
> Index: gcc/testsuite/gcc.target/powerpc/safe-indirect-jump-8.c
> ===================================================================
> --- gcc/testsuite/gcc.target/powerpc/safe-indirect-jump-8.c	(revision 256894)
> +++ gcc/testsuite/gcc.target/powerpc/safe-indirect-jump-8.c	(working copy)
> @@ -12,4 +12,4 @@ int bar ()
> 
> /* { dg-final { scan-assembler "crset eq" } } */
> /* { dg-final { scan-assembler "beqctr-" } } */
> -/* { dg-final { scan-assembler "b ." } } */
> +/* { dg-final { scan-assembler "b $" } } */
>
Jakub Jelinek Jan. 19, 2018, 9:09 p.m. UTC | #2
On Fri, Jan 19, 2018 at 02:58:07PM -0600, Bill Schmidt wrote:
> --- gcc/testsuite/gcc.target/powerpc/safe-indirect-jump-2.c	(revision 256894)
> +++ gcc/testsuite/gcc.target/powerpc/safe-indirect-jump-2.c	(working copy)
> @@ -30,4 +30,4 @@ int foo (int x)
>  
>  /* { dg-final { scan-assembler "crset 30" } } */
>  /* { dg-final { scan-assembler "beqctr- 7" } } */
> -/* { dg-final { scan-assembler "b ." } } */
> +/* { dg-final { scan-assembler "b $" } } */

Does $ in scan-assembler really match a literal $ and not end of line?
Looking around, most of scan-assembler patterns that want to match a $ use
\\\$

> Index: gcc/testsuite/gcc.target/powerpc/safe-indirect-jump-3.c
> ===================================================================
> --- gcc/testsuite/gcc.target/powerpc/safe-indirect-jump-3.c	(revision 256894)
> +++ gcc/testsuite/gcc.target/powerpc/safe-indirect-jump-3.c	(working copy)
> @@ -49,4 +49,4 @@ int foo (int x)
>  
>  /* { dg-final { scan-assembler "crset 30" } } */
>  /* { dg-final { scan-assembler "beqctr- 7" } } */
> -/* { dg-final { scan-assembler "b ." } } */
> +/* { dg-final { scan-assembler "b $" } } */
> Index: gcc/testsuite/gcc.target/powerpc/safe-indirect-jump-8.c
> ===================================================================
> --- gcc/testsuite/gcc.target/powerpc/safe-indirect-jump-8.c	(revision 256894)
> +++ gcc/testsuite/gcc.target/powerpc/safe-indirect-jump-8.c	(working copy)
> @@ -12,4 +12,4 @@ int bar ()
>  
>  /* { dg-final { scan-assembler "crset eq" } } */
>  /* { dg-final { scan-assembler "beqctr-" } } */
> -/* { dg-final { scan-assembler "b ." } } */
> +/* { dg-final { scan-assembler "b $" } } */
> Index: gcc/testsuite/gcc.target/powerpc/safe-indirect-jump-2.c
> ===================================================================
> --- gcc/testsuite/gcc.target/powerpc/safe-indirect-jump-2.c	(revision 256894)
> +++ gcc/testsuite/gcc.target/powerpc/safe-indirect-jump-2.c	(working copy)
> @@ -30,4 +30,4 @@ int foo (int x)
>  
>  /* { dg-final { scan-assembler "crset 30" } } */
>  /* { dg-final { scan-assembler "beqctr- 7" } } */
> -/* { dg-final { scan-assembler "b ." } } */
> +/* { dg-final { scan-assembler "b $" } } */
> Index: gcc/testsuite/gcc.target/powerpc/safe-indirect-jump-3.c
> ===================================================================
> --- gcc/testsuite/gcc.target/powerpc/safe-indirect-jump-3.c	(revision 256894)
> +++ gcc/testsuite/gcc.target/powerpc/safe-indirect-jump-3.c	(working copy)
> @@ -49,4 +49,4 @@ int foo (int x)
>  
>  /* { dg-final { scan-assembler "crset 30" } } */
>  /* { dg-final { scan-assembler "beqctr- 7" } } */
> -/* { dg-final { scan-assembler "b ." } } */
> +/* { dg-final { scan-assembler "b $" } } */
> Index: gcc/testsuite/gcc.target/powerpc/safe-indirect-jump-8.c
> ===================================================================
> --- gcc/testsuite/gcc.target/powerpc/safe-indirect-jump-8.c	(revision 256894)
> +++ gcc/testsuite/gcc.target/powerpc/safe-indirect-jump-8.c	(working copy)
> @@ -12,4 +12,4 @@ int bar ()
>  
>  /* { dg-final { scan-assembler "crset eq" } } */
>  /* { dg-final { scan-assembler "beqctr-" } } */
> -/* { dg-final { scan-assembler "b ." } } */
> +/* { dg-final { scan-assembler "b $" } } */

	Jakub
Bill Schmidt Jan. 19, 2018, 9:13 p.m. UTC | #3
> On Jan 19, 2018, at 3:09 PM, Jakub Jelinek <jakub@redhat.com> wrote:
> 
> On Fri, Jan 19, 2018 at 02:58:07PM -0600, Bill Schmidt wrote:
>> --- gcc/testsuite/gcc.target/powerpc/safe-indirect-jump-2.c	(revision 256894)
>> +++ gcc/testsuite/gcc.target/powerpc/safe-indirect-jump-2.c	(working copy)
>> @@ -30,4 +30,4 @@ int foo (int x)
>> 
>> /* { dg-final { scan-assembler "crset 30" } } */
>> /* { dg-final { scan-assembler "beqctr- 7" } } */
>> -/* { dg-final { scan-assembler "b ." } } */
>> +/* { dg-final { scan-assembler "b $" } } */
> 
> Does $ in scan-assembler really match a literal $ and not end of line?
> Looking around, most of scan-assembler patterns that want to match a $ use
> \\\$

Right.  Working on getting the right number of backslashes in here...
I can never remember which ones need one and which need 3.

Thanks...

Bill
> 
>> Index: gcc/testsuite/gcc.target/powerpc/safe-indirect-jump-3.c
>> ===================================================================
>> --- gcc/testsuite/gcc.target/powerpc/safe-indirect-jump-3.c	(revision 256894)
>> +++ gcc/testsuite/gcc.target/powerpc/safe-indirect-jump-3.c	(working copy)
>> @@ -49,4 +49,4 @@ int foo (int x)
>> 
>> /* { dg-final { scan-assembler "crset 30" } } */
>> /* { dg-final { scan-assembler "beqctr- 7" } } */
>> -/* { dg-final { scan-assembler "b ." } } */
>> +/* { dg-final { scan-assembler "b $" } } */
>> Index: gcc/testsuite/gcc.target/powerpc/safe-indirect-jump-8.c
>> ===================================================================
>> --- gcc/testsuite/gcc.target/powerpc/safe-indirect-jump-8.c	(revision 256894)
>> +++ gcc/testsuite/gcc.target/powerpc/safe-indirect-jump-8.c	(working copy)
>> @@ -12,4 +12,4 @@ int bar ()
>> 
>> /* { dg-final { scan-assembler "crset eq" } } */
>> /* { dg-final { scan-assembler "beqctr-" } } */
>> -/* { dg-final { scan-assembler "b ." } } */
>> +/* { dg-final { scan-assembler "b $" } } */
>> Index: gcc/testsuite/gcc.target/powerpc/safe-indirect-jump-2.c
>> ===================================================================
>> --- gcc/testsuite/gcc.target/powerpc/safe-indirect-jump-2.c	(revision 256894)
>> +++ gcc/testsuite/gcc.target/powerpc/safe-indirect-jump-2.c	(working copy)
>> @@ -30,4 +30,4 @@ int foo (int x)
>> 
>> /* { dg-final { scan-assembler "crset 30" } } */
>> /* { dg-final { scan-assembler "beqctr- 7" } } */
>> -/* { dg-final { scan-assembler "b ." } } */
>> +/* { dg-final { scan-assembler "b $" } } */
>> Index: gcc/testsuite/gcc.target/powerpc/safe-indirect-jump-3.c
>> ===================================================================
>> --- gcc/testsuite/gcc.target/powerpc/safe-indirect-jump-3.c	(revision 256894)
>> +++ gcc/testsuite/gcc.target/powerpc/safe-indirect-jump-3.c	(working copy)
>> @@ -49,4 +49,4 @@ int foo (int x)
>> 
>> /* { dg-final { scan-assembler "crset 30" } } */
>> /* { dg-final { scan-assembler "beqctr- 7" } } */
>> -/* { dg-final { scan-assembler "b ." } } */
>> +/* { dg-final { scan-assembler "b $" } } */
>> Index: gcc/testsuite/gcc.target/powerpc/safe-indirect-jump-8.c
>> ===================================================================
>> --- gcc/testsuite/gcc.target/powerpc/safe-indirect-jump-8.c	(revision 256894)
>> +++ gcc/testsuite/gcc.target/powerpc/safe-indirect-jump-8.c	(working copy)
>> @@ -12,4 +12,4 @@ int bar ()
>> 
>> /* { dg-final { scan-assembler "crset eq" } } */
>> /* { dg-final { scan-assembler "beqctr-" } } */
>> -/* { dg-final { scan-assembler "b ." } } */
>> +/* { dg-final { scan-assembler "b $" } } */
> 
> 	Jakub
Andreas Schwab Jan. 19, 2018, 9:20 p.m. UTC | #4
On Jan 19 2018, Bill Schmidt <wschmidt@linux.vnet.ibm.com> wrote:

>> On Jan 19, 2018, at 3:09 PM, Jakub Jelinek <jakub@redhat.com> wrote:
>> 
>> On Fri, Jan 19, 2018 at 02:58:07PM -0600, Bill Schmidt wrote:
>>> --- gcc/testsuite/gcc.target/powerpc/safe-indirect-jump-2.c	(revision 256894)
>>> +++ gcc/testsuite/gcc.target/powerpc/safe-indirect-jump-2.c	(working copy)
>>> @@ -30,4 +30,4 @@ int foo (int x)
>>> 
>>> /* { dg-final { scan-assembler "crset 30" } } */
>>> /* { dg-final { scan-assembler "beqctr- 7" } } */
>>> -/* { dg-final { scan-assembler "b ." } } */
>>> +/* { dg-final { scan-assembler "b $" } } */
>> 
>> Does $ in scan-assembler really match a literal $ and not end of line?
>> Looking around, most of scan-assembler patterns that want to match a $ use
>> \\\$
>
> Right.  Working on getting the right number of backslashes in here...
> I can never remember which ones need one and which need 3.

Use braces.

Andreas.
Segher Boessenkool Jan. 19, 2018, 9:58 p.m. UTC | #5
On Fri, Jan 19, 2018 at 10:20:23PM +0100, Andreas Schwab wrote:
> On Jan 19 2018, Bill Schmidt <wschmidt@linux.vnet.ibm.com> wrote:
> 
> >> On Jan 19, 2018, at 3:09 PM, Jakub Jelinek <jakub@redhat.com> wrote:
> >> 
> >> On Fri, Jan 19, 2018 at 02:58:07PM -0600, Bill Schmidt wrote:
> >>> --- gcc/testsuite/gcc.target/powerpc/safe-indirect-jump-2.c	(revision 256894)
> >>> +++ gcc/testsuite/gcc.target/powerpc/safe-indirect-jump-2.c	(working copy)
> >>> @@ -30,4 +30,4 @@ int foo (int x)
> >>> 
> >>> /* { dg-final { scan-assembler "crset 30" } } */
> >>> /* { dg-final { scan-assembler "beqctr- 7" } } */
> >>> -/* { dg-final { scan-assembler "b ." } } */
> >>> +/* { dg-final { scan-assembler "b $" } } */
> >> 
> >> Does $ in scan-assembler really match a literal $ and not end of line?
> >> Looking around, most of scan-assembler patterns that want to match a $ use
> >> \\\$
> >
> > Right.  Working on getting the right number of backslashes in here...
> > I can never remember which ones need one and which need 3.
> 
> Use braces.

Yes, either

+/* { dg-final { scan-assembler "b \\\$" } } */

or (preferably)

+/* { dg-final { scan-assembler {b \$} } } */


Segher
Bill Schmidt Jan. 19, 2018, 10:35 p.m. UTC | #6
> On Jan 19, 2018, at 3:58 PM, Segher Boessenkool <segher@kernel.crashing.org> wrote:
> 
> On Fri, Jan 19, 2018 at 10:20:23PM +0100, Andreas Schwab wrote:
>> On Jan 19 2018, Bill Schmidt <wschmidt@linux.vnet.ibm.com> wrote:
>> 
>>>> On Jan 19, 2018, at 3:09 PM, Jakub Jelinek <jakub@redhat.com> wrote:
>>>> 
>>>> On Fri, Jan 19, 2018 at 02:58:07PM -0600, Bill Schmidt wrote:
>>>>> --- gcc/testsuite/gcc.target/powerpc/safe-indirect-jump-2.c	(revision 256894)
>>>>> +++ gcc/testsuite/gcc.target/powerpc/safe-indirect-jump-2.c	(working copy)
>>>>> @@ -30,4 +30,4 @@ int foo (int x)
>>>>> 
>>>>> /* { dg-final { scan-assembler "crset 30" } } */
>>>>> /* { dg-final { scan-assembler "beqctr- 7" } } */
>>>>> -/* { dg-final { scan-assembler "b ." } } */
>>>>> +/* { dg-final { scan-assembler "b $" } } */
>>>> 
>>>> Does $ in scan-assembler really match a literal $ and not end of line?
>>>> Looking around, most of scan-assembler patterns that want to match a $ use
>>>> \\\$
>>> 
>>> Right.  Working on getting the right number of backslashes in here...
>>> I can never remember which ones need one and which need 3.
>> 
>> Use braces.
> 
> Yes, either
> 
> +/* { dg-final { scan-assembler "b \\\$" } } */
> 
> or (preferably)
> 
> +/* { dg-final { scan-assembler {b \$} } } */

Thanks, going with the latter suggestion.  My environment crapped out so I'm restarting
the regstrap.  Will repost later this evening after it passes.

Thanks,
Bill
> 
> 
> Segher
David Edelsohn Jan. 20, 2018, 1:53 a.m. UTC | #7
On Fri, Jan 19, 2018 at 3:58 PM, Bill Schmidt
<wschmidt@linux.vnet.ibm.com> wrote:
> Hi,
>
> My recent patches to trunk and gcc-7-branch for avoiding speculation of
> indirect branches has a flaw, pointed out by David.  Usage of "." to
> represent the program counter is not portable across all POWER
> assemblers, particularly not being accepted on AIX.  "$" is the
> universally accepted alternative.  So change the code and the test
> cases to use $ instead of . for this purpose.
>
> Regstrap is in progress on powerpc64-linux-gnu and powerpc64le-linux-gnu.
> Assuming no issues are found, is this okay for trunk and backport to 7?

Once I got past the "." issue, I have discovered that the AIX
assembler also doesn't like

crset eq

It doesn't like the symbolic name for the operand, it wants a numeric
operand for "eq".

Thanks, David
Segher Boessenkool Jan. 20, 2018, 2:05 a.m. UTC | #8
On Fri, Jan 19, 2018 at 08:53:52PM -0500, David Edelsohn wrote:
> On Fri, Jan 19, 2018 at 3:58 PM, Bill Schmidt
> <wschmidt@linux.vnet.ibm.com> wrote:
> > Hi,
> >
> > My recent patches to trunk and gcc-7-branch for avoiding speculation of
> > indirect branches has a flaw, pointed out by David.  Usage of "." to
> > represent the program counter is not portable across all POWER
> > assemblers, particularly not being accepted on AIX.  "$" is the
> > universally accepted alternative.  So change the code and the test
> > cases to use $ instead of . for this purpose.
> >
> > Regstrap is in progress on powerpc64-linux-gnu and powerpc64le-linux-gnu.
> > Assuming no issues are found, is this okay for trunk and backport to 7?
> 
> Once I got past the "." issue, I have discovered that the AIX
> assembler also doesn't like
> 
> crset eq
> 
> It doesn't like the symbolic name for the operand, it wants a numeric
> operand for "eq".

Wow, nasty.  It is the very first thing in the "extended mnemonics"
appendix in the ISA.

"crset 2" works (I tried it out on gcc111).


Segher
Bill Schmidt Jan. 20, 2018, 2:06 a.m. UTC | #9
> On Jan 19, 2018, at 7:53 PM, David Edelsohn <dje.gcc@gmail.com> wrote:
> 
> On Fri, Jan 19, 2018 at 3:58 PM, Bill Schmidt
> <wschmidt@linux.vnet.ibm.com> wrote:
>> Hi,
>> 
>> My recent patches to trunk and gcc-7-branch for avoiding speculation of
>> indirect branches has a flaw, pointed out by David.  Usage of "." to
>> represent the program counter is not portable across all POWER
>> assemblers, particularly not being accepted on AIX.  "$" is the
>> universally accepted alternative.  So change the code and the test
>> cases to use $ instead of . for this purpose.
>> 
>> Regstrap is in progress on powerpc64-linux-gnu and powerpc64le-linux-gnu.
>> Assuming no issues are found, is this okay for trunk and backport to 7?
> 
> Once I got past the "." issue, I have discovered that the AIX
> assembler also doesn't like
> 
> crset eq
> 
> It doesn't like the symbolic name for the operand, it wants a numeric
> operand for "eq".

This seems to me to be a different matter.  The ISA defines this syntax
in Appendix C and specifies that assemblers should provide everything
listed there.  The use of eq standing alone is shown in the examples in
section C.3.  So the AIX assembler is noncompliant in this case.  I
realize that as a practical matter we have to deal with that, but this
needs to be fixed at some point.  Where do we file the bug report?

I'm having a lot of heartburn over this because my test machine is
experiencing disk slowdowns, so it's taking me up to 4 hours to complete
a bootstrap and regression test.  So whenever my current test finishes
I plan to repost the patch in existing form so that it can be committed
tomorrow.

I'll try to set up another patch later tonight if I don't completely run out
of gas so it can burn overnight.  But frankly I may not manage it.

Bill
> 
> Thanks, David
>
Mike Stump Jan. 20, 2018, 8:10 a.m. UTC | #10
On Jan 19, 2018, at 6:06 PM, Bill Schmidt <wschmidt@linux.vnet.ibm.com> wrote:
> 
> I'm having a lot of heartburn over this because my test machine is
> experiencing disk slowdowns, so it's taking me up to 4 hours to complete
> a bootstrap and regression test.

Ah, the joys of crosses, no bootstrap.  The gcc C testsuite runs in under 120 seconds for me.  [ ducks ]
diff mbox series

Patch

Index: gcc/config/rs6000/rs6000.md
===================================================================
--- gcc/config/rs6000/rs6000.md	(revision 256894)
+++ gcc/config/rs6000/rs6000.md	(working copy)
@@ -10987,7 +10987,7 @@ 
 	return \"b%T0\";
       else
 	/* Can use CR0 since it is volatile across sibcalls.  */
-	return \"crset eq\;beq%T0-\;b .\";
+	return \"crset eq\;beq%T0-\;b $\";
     }
   else if (DEFAULT_ABI == ABI_V4 && flag_pic)
     {
@@ -11044,7 +11044,7 @@ 
 	return \"b%T1\";
       else
 	/* Can use CR0 since it is volatile across sibcalls.  */
-	return \"crset eq\;beq%T1-\;b .\";
+	return \"crset eq\;beq%T1-\;b $\";
     }
   else if (DEFAULT_ABI == ABI_V4 && flag_pic)
     {
@@ -12566,7 +12566,7 @@ 
   [(set (pc) (match_operand:P 0 "register_operand" "c,*l"))
    (clobber (match_operand:CC 1 "cc_reg_operand" "=y,y"))]
   "!rs6000_speculate_indirect_jumps"
-  "crset %E1\;beq%T0- %1\;b ."
+  "crset %E1\;beq%T0- %1\;b $"
   [(set_attr "type" "jmpreg")
    (set_attr "length" "12")])
 
@@ -12672,7 +12672,7 @@ 
    (use (label_ref (match_operand 1)))
    (clobber (match_operand:CC 2 "cc_reg_operand" "=y,y"))]
   "!rs6000_speculate_indirect_jumps"
-  "crset %E2\;beq%T0- %2\;b ."
+  "crset %E2\;beq%T0- %2\;b $"
   [(set_attr "type" "jmpreg")
    (set_attr "length" "12")])
 
Index: gcc/testsuite/gcc.target/powerpc/safe-indirect-jump-2.c
===================================================================
--- gcc/testsuite/gcc.target/powerpc/safe-indirect-jump-2.c	(revision 256894)
+++ gcc/testsuite/gcc.target/powerpc/safe-indirect-jump-2.c	(working copy)
@@ -30,4 +30,4 @@  int foo (int x)
 
 /* { dg-final { scan-assembler "crset 30" } } */
 /* { dg-final { scan-assembler "beqctr- 7" } } */
-/* { dg-final { scan-assembler "b ." } } */
+/* { dg-final { scan-assembler "b $" } } */
Index: gcc/testsuite/gcc.target/powerpc/safe-indirect-jump-3.c
===================================================================
--- gcc/testsuite/gcc.target/powerpc/safe-indirect-jump-3.c	(revision 256894)
+++ gcc/testsuite/gcc.target/powerpc/safe-indirect-jump-3.c	(working copy)
@@ -49,4 +49,4 @@  int foo (int x)
 
 /* { dg-final { scan-assembler "crset 30" } } */
 /* { dg-final { scan-assembler "beqctr- 7" } } */
-/* { dg-final { scan-assembler "b ." } } */
+/* { dg-final { scan-assembler "b $" } } */
Index: gcc/testsuite/gcc.target/powerpc/safe-indirect-jump-8.c
===================================================================
--- gcc/testsuite/gcc.target/powerpc/safe-indirect-jump-8.c	(revision 256894)
+++ gcc/testsuite/gcc.target/powerpc/safe-indirect-jump-8.c	(working copy)
@@ -12,4 +12,4 @@  int bar ()
 
 /* { dg-final { scan-assembler "crset eq" } } */
 /* { dg-final { scan-assembler "beqctr-" } } */
-/* { dg-final { scan-assembler "b ." } } */
+/* { dg-final { scan-assembler "b $" } } */
Index: gcc/config/rs6000/rs6000.md
===================================================================
--- gcc/config/rs6000/rs6000.md	(revision 256894)
+++ gcc/config/rs6000/rs6000.md	(working copy)
@@ -10987,7 +10987,7 @@ 
 	return \"b%T0\";
       else
 	/* Can use CR0 since it is volatile across sibcalls.  */
-	return \"crset eq\;beq%T0-\;b .\";
+	return \"crset eq\;beq%T0-\;b $\";
     }
   else if (DEFAULT_ABI == ABI_V4 && flag_pic)
     {
@@ -11044,7 +11044,7 @@ 
 	return \"b%T1\";
       else
 	/* Can use CR0 since it is volatile across sibcalls.  */
-	return \"crset eq\;beq%T1-\;b .\";
+	return \"crset eq\;beq%T1-\;b $\";
     }
   else if (DEFAULT_ABI == ABI_V4 && flag_pic)
     {
@@ -12566,7 +12566,7 @@ 
   [(set (pc) (match_operand:P 0 "register_operand" "c,*l"))
    (clobber (match_operand:CC 1 "cc_reg_operand" "=y,y"))]
   "!rs6000_speculate_indirect_jumps"
-  "crset %E1\;beq%T0- %1\;b ."
+  "crset %E1\;beq%T0- %1\;b $"
   [(set_attr "type" "jmpreg")
    (set_attr "length" "12")])
 
@@ -12672,7 +12672,7 @@ 
    (use (label_ref (match_operand 1)))
    (clobber (match_operand:CC 2 "cc_reg_operand" "=y,y"))]
   "!rs6000_speculate_indirect_jumps"
-  "crset %E2\;beq%T0- %2\;b ."
+  "crset %E2\;beq%T0- %2\;b $"
   [(set_attr "type" "jmpreg")
    (set_attr "length" "12")])
 
Index: gcc/testsuite/gcc.target/powerpc/safe-indirect-jump-2.c
===================================================================
--- gcc/testsuite/gcc.target/powerpc/safe-indirect-jump-2.c	(revision 256894)
+++ gcc/testsuite/gcc.target/powerpc/safe-indirect-jump-2.c	(working copy)
@@ -30,4 +30,4 @@  int foo (int x)
 
 /* { dg-final { scan-assembler "crset 30" } } */
 /* { dg-final { scan-assembler "beqctr- 7" } } */
-/* { dg-final { scan-assembler "b ." } } */
+/* { dg-final { scan-assembler "b $" } } */
Index: gcc/testsuite/gcc.target/powerpc/safe-indirect-jump-3.c
===================================================================
--- gcc/testsuite/gcc.target/powerpc/safe-indirect-jump-3.c	(revision 256894)
+++ gcc/testsuite/gcc.target/powerpc/safe-indirect-jump-3.c	(working copy)
@@ -49,4 +49,4 @@  int foo (int x)
 
 /* { dg-final { scan-assembler "crset 30" } } */
 /* { dg-final { scan-assembler "beqctr- 7" } } */
-/* { dg-final { scan-assembler "b ." } } */
+/* { dg-final { scan-assembler "b $" } } */
Index: gcc/testsuite/gcc.target/powerpc/safe-indirect-jump-8.c
===================================================================
--- gcc/testsuite/gcc.target/powerpc/safe-indirect-jump-8.c	(revision 256894)
+++ gcc/testsuite/gcc.target/powerpc/safe-indirect-jump-8.c	(working copy)
@@ -12,4 +12,4 @@  int bar ()
 
 /* { dg-final { scan-assembler "crset eq" } } */
 /* { dg-final { scan-assembler "beqctr-" } } */
-/* { dg-final { scan-assembler "b ." } } */
+/* { dg-final { scan-assembler "b $" } } */