diff mbox series

x32: Encode %esp as %rsp to avoid 0x67 prefix

Message ID 20170923211620.GA2652@gmail.com
State New
Headers show
Series x32: Encode %esp as %rsp to avoid 0x67 prefix | expand

Commit Message

H.J. Lu Sept. 23, 2017, 9:16 p.m. UTC 
Since the upper 32 bits of stack register are always zero for x32, we
can encode %esp as %rsp to avoid 0x67 prefix in address if there is no
index or base register.

Tested on x86-64.  OK for trunk?

H.J.
----
gcc/

	PR target/82267
	* config/i386/i386.c (ix86_print_operand_address_as): Encode
	%esp as %rsp to avoid 0x67 prefix if there is no index or base
	register.

gcc/testsuite/

	PR target/82267
	* gcc.target/i386/pr82267.c: New tests.
---
 gcc/config/i386/i386.c                  | 17 +++++++++++++++++
 gcc/testsuite/gcc.target/i386/pr82267.c | 14 ++++++++++++++
 2 files changed, 31 insertions(+)
 create mode 100644 gcc/testsuite/gcc.target/i386/pr82267.c

Comments

Uros Bizjak Sept. 24, 2017, 9:25 a.m. UTC | #1 
On Sat, Sep 23, 2017 at 11:16 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
> Since the upper 32 bits of stack register are always zero for x32, we
> can encode %esp as %rsp to avoid 0x67 prefix in address if there is no
> index or base register.
>
> Tested on x86-64.  OK for trunk?
>
> H.J.
> ----
> gcc/
>
>         PR target/82267
>         * config/i386/i386.c (ix86_print_operand_address_as): Encode
>         %esp as %rsp to avoid 0x67 prefix if there is no index or base
>         register.
>
> gcc/testsuite/
>
>         PR target/82267
>         * gcc.target/i386/pr82267.c: New tests.
> ---
>  gcc/config/i386/i386.c                  | 17 +++++++++++++++++
>  gcc/testsuite/gcc.target/i386/pr82267.c | 14 ++++++++++++++
>  2 files changed, 31 insertions(+)
>  create mode 100644 gcc/testsuite/gcc.target/i386/pr82267.c
>
> diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
> index 5e8f58c5e9f..519fdb0ffae 100644
> --- a/gcc/config/i386/i386.c
> +++ b/gcc/config/i386/i386.c
> @@ -19849,6 +19849,23 @@ ix86_print_operand_address_as (FILE *file, rtx addr,
>    disp = parts.disp;
>    scale = parts.scale;
>
> +  /* Since the upper 32 bits of RSP are always zero for x32, we can
> +     encode %esp as %rsp to avoid 0x67 prefix if there is no index or
> +     base register.  */
> +  if (TARGET_X32
> +      && Pmode == SImode
> +      && (!index || !base)
> +      && index != base)
> +    {
> +      if (base)
> +       {
> +         if (REGNO (base) == SP_REG)
> +           base = gen_rtx_REG (DImode, SP_REG);
> +       }
> +      else if (REGNO (index) == SP_REG)
> +       index = gen_rtx_REG (DImode, SP_REG);
> +    }
> +

We can use 'q' modifier just before register output part (plus a small
simplification).

Can you try the attached (untested) patch?

Uros.
Index: i386.c
===================================================================
--- i386.c	(revision 253118)
+++ i386.c	(working copy)
@@ -19953,6 +19953,14 @@ ix86_print_operand_address_as (FILE *file, rtx add
 	  code = 'k';
 	}
 
+      /* Since the upper 32 bits of RSP are always zero for x32, we can
+	 encode %esp as %rsp to avoid 0x67 prefix if there is no index or
+	 base register.  */
+      if (TARGET_X32 && Pmode == SImode
+	  && ((!index && base && REGNO (base) == SP_REG)
+	      || (!base && index && REGNO (index) == SP_REG)))
+	code = 'q';
+
       if (ASSEMBLER_DIALECT == ASM_ATT)
 	{
 	  if (disp)
H.J. Lu Sept. 24, 2017, 9:45 a.m. UTC | #2 
On 9/24/17, Uros Bizjak <ubizjak@gmail.com> wrote:
> On Sat, Sep 23, 2017 at 11:16 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
>> Since the upper 32 bits of stack register are always zero for x32, we
>> can encode %esp as %rsp to avoid 0x67 prefix in address if there is no
>> index or base register.
>>
>> Tested on x86-64.  OK for trunk?
>>
>> H.J.
>> ----
>> gcc/
>>
>>         PR target/82267
>>         * config/i386/i386.c (ix86_print_operand_address_as): Encode
>>         %esp as %rsp to avoid 0x67 prefix if there is no index or base
>>         register.
>>
>> gcc/testsuite/
>>
>>         PR target/82267
>>         * gcc.target/i386/pr82267.c: New tests.
>> ---
>>  gcc/config/i386/i386.c                  | 17 +++++++++++++++++
>>  gcc/testsuite/gcc.target/i386/pr82267.c | 14 ++++++++++++++
>>  2 files changed, 31 insertions(+)
>>  create mode 100644 gcc/testsuite/gcc.target/i386/pr82267.c
>>
>> diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
>> index 5e8f58c5e9f..519fdb0ffae 100644
>> --- a/gcc/config/i386/i386.c
>> +++ b/gcc/config/i386/i386.c
>> @@ -19849,6 +19849,23 @@ ix86_print_operand_address_as (FILE *file, rtx
>> addr,
>>    disp = parts.disp;
>>    scale = parts.scale;
>>
>> +  /* Since the upper 32 bits of RSP are always zero for x32, we can
>> +     encode %esp as %rsp to avoid 0x67 prefix if there is no index or
>> +     base register.  */
>> +  if (TARGET_X32
>> +      && Pmode == SImode
>> +      && (!index || !base)
>> +      && index != base)
>> +    {
>> +      if (base)
>> +       {
>> +         if (REGNO (base) == SP_REG)
>> +           base = gen_rtx_REG (DImode, SP_REG);
>> +       }
>> +      else if (REGNO (index) == SP_REG)
>> +       index = gen_rtx_REG (DImode, SP_REG);
>> +    }
>> +
>
> We can use 'q' modifier just before register output part (plus a small
> simplification).
>
> Can you try the attached (untested) patch?
>

Yes, it works.

Thanks.
Jakub Jelinek Sept. 26, 2017, 12:19 p.m. UTC | #3 
On Sun, Sep 24, 2017 at 11:25:34AM +0200, Uros Bizjak wrote:
> We can use 'q' modifier just before register output part (plus a small
> simplification).
> 
> Can you try the attached (untested) patch?
> 
> Uros.

> Index: i386.c
> ===================================================================
> --- i386.c	(revision 253118)
> +++ i386.c	(working copy)
> @@ -19953,6 +19953,14 @@ ix86_print_operand_address_as (FILE *file, rtx add
>  	  code = 'k';
>  	}
>  
> +      /* Since the upper 32 bits of RSP are always zero for x32, we can
> +	 encode %esp as %rsp to avoid 0x67 prefix if there is no index or
> +	 base register.  */
> +      if (TARGET_X32 && Pmode == SImode
> +	  && ((!index && base && REGNO (base) == SP_REG)
> +	      || (!base && index && REGNO (index) == SP_REG)))
> +	code = 'q';
> +
>        if (ASSEMBLER_DIALECT == ASM_ATT)
>  	{
>  	  if (disp)

This broke
+FAIL: gcc.target/i386/pr55049-1.c (internal compiler error)
+FAIL: gcc.target/i386/pr55049-1.c (test for excess errors)
+FAIL: gcc.target/i386/pr59034-1.c (internal compiler error)
+FAIL: gcc.target/i386/pr59034-1.c (test for excess errors)
+FAIL: g++.dg/other/pr59492.C  -std=gnu++11 (internal compiler error)
+FAIL: g++.dg/other/pr59492.C  -std=gnu++11 (test for excess errors)
+FAIL: g++.dg/other/pr59492.C  -std=gnu++14 (internal compiler error)
+FAIL: g++.dg/other/pr59492.C  -std=gnu++14 (test for excess errors)
+FAIL: g++.dg/other/pr59492.C  -std=gnu++98 (internal compiler error)
+FAIL: g++.dg/other/pr59492.C  -std=gnu++98 (test for excess errors)
with --enable-checking=yes,rtl,extra.

While I believe non-NULL index must be a REG, that is not the case for base,
which can be pc_rtx.  A few lines later it calls print_reg on base and/or
index (if non-NULL) and that assumes it is pc_rtx or uses REGNO on it,
so I think we can't see a SUBREG or something similar there.

Fixed thusly, ok for trunk?

2017-09-26  Jakub Jelinek  <jakub@redhat.com>

	PR target/82267
	* config/i386/i386.c (ix86_print_operand_address_as): Only test
	REGNO (base) == SP_REG if base is a REG.

--- gcc/config/i386/i386.c.jj	2017-09-26 11:38:54.000000000 +0200
+++ gcc/config/i386/i386.c	2017-09-26 14:04:51.444459554 +0200
@@ -19957,7 +19957,7 @@ ix86_print_operand_address_as (FILE *fil
 	 encode %esp as %rsp to avoid 0x67 prefix if there is no index or
 	 base register.  */
       if (TARGET_X32 && Pmode == SImode
-	  && ((!index && base && REGNO (base) == SP_REG)
+	  && ((!index && base && REG_P (base) && REGNO (base) == SP_REG)
 	      || (!base && index && REGNO (index) == SP_REG)))
 	code = 'q';
 
 

	Jakub
Uros Bizjak Sept. 26, 2017, 12:24 p.m. UTC | #4 
On Tue, Sep 26, 2017 at 2:19 PM, Jakub Jelinek <jakub@redhat.com> wrote:
> On Sun, Sep 24, 2017 at 11:25:34AM +0200, Uros Bizjak wrote:
>> We can use 'q' modifier just before register output part (plus a small
>> simplification).
>>
>> Can you try the attached (untested) patch?
>>
>> Uros.
>
>> Index: i386.c
>> ===================================================================
>> --- i386.c    (revision 253118)
>> +++ i386.c    (working copy)
>> @@ -19953,6 +19953,14 @@ ix86_print_operand_address_as (FILE *file, rtx add
>>         code = 'k';
>>       }
>>
>> +      /* Since the upper 32 bits of RSP are always zero for x32, we can
>> +      encode %esp as %rsp to avoid 0x67 prefix if there is no index or
>> +      base register.  */
>> +      if (TARGET_X32 && Pmode == SImode
>> +       && ((!index && base && REGNO (base) == SP_REG)
>> +           || (!base && index && REGNO (index) == SP_REG)))
>> +     code = 'q';
>> +
>>        if (ASSEMBLER_DIALECT == ASM_ATT)
>>       {
>>         if (disp)
>
> This broke
> +FAIL: gcc.target/i386/pr55049-1.c (internal compiler error)
> +FAIL: gcc.target/i386/pr55049-1.c (test for excess errors)
> +FAIL: gcc.target/i386/pr59034-1.c (internal compiler error)
> +FAIL: gcc.target/i386/pr59034-1.c (test for excess errors)
> +FAIL: g++.dg/other/pr59492.C  -std=gnu++11 (internal compiler error)
> +FAIL: g++.dg/other/pr59492.C  -std=gnu++11 (test for excess errors)
> +FAIL: g++.dg/other/pr59492.C  -std=gnu++14 (internal compiler error)
> +FAIL: g++.dg/other/pr59492.C  -std=gnu++14 (test for excess errors)
> +FAIL: g++.dg/other/pr59492.C  -std=gnu++98 (internal compiler error)
> +FAIL: g++.dg/other/pr59492.C  -std=gnu++98 (test for excess errors)
> with --enable-checking=yes,rtl,extra.
>
> While I believe non-NULL index must be a REG, that is not the case for base,
> which can be pc_rtx.  A few lines later it calls print_reg on base and/or
> index (if non-NULL) and that assumes it is pc_rtx or uses REGNO on it,
> so I think we can't see a SUBREG or something similar there.
>
> Fixed thusly, ok for trunk?
>
> 2017-09-26  Jakub Jelinek  <jakub@redhat.com>
>
>         PR target/82267
>         * config/i386/i386.c (ix86_print_operand_address_as): Only test
>         REGNO (base) == SP_REG if base is a REG.

OK.

Thanks,
Uros.

> --- gcc/config/i386/i386.c.jj   2017-09-26 11:38:54.000000000 +0200
> +++ gcc/config/i386/i386.c      2017-09-26 14:04:51.444459554 +0200
> @@ -19957,7 +19957,7 @@ ix86_print_operand_address_as (FILE *fil
>          encode %esp as %rsp to avoid 0x67 prefix if there is no index or
>          base register.  */
>        if (TARGET_X32 && Pmode == SImode
> -         && ((!index && base && REGNO (base) == SP_REG)
> +         && ((!index && base && REG_P (base) && REGNO (base) == SP_REG)
>               || (!base && index && REGNO (index) == SP_REG)))
>         code = 'q';
>
>
>
>         Jakub
diff mbox series

Patch

diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index 5e8f58c5e9f..519fdb0ffae 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -19849,6 +19849,23 @@  ix86_print_operand_address_as (FILE *file, rtx addr,
   disp = parts.disp;
   scale = parts.scale;
 
+  /* Since the upper 32 bits of RSP are always zero for x32, we can
+     encode %esp as %rsp to avoid 0x67 prefix if there is no index or
+     base register.  */
+  if (TARGET_X32
+      && Pmode == SImode
+      && (!index || !base)
+      && index != base)
+    {
+      if (base)
+	{
+	  if (REGNO (base) == SP_REG)
+	    base = gen_rtx_REG (DImode, SP_REG);
+	}
+      else if (REGNO (index) == SP_REG)
+	index = gen_rtx_REG (DImode, SP_REG);
+    }
+
   if (ADDR_SPACE_GENERIC_P (as))
     as = parts.seg;
   else
diff --git a/gcc/testsuite/gcc.target/i386/pr82267.c b/gcc/testsuite/gcc.target/i386/pr82267.c
new file mode 100644
index 00000000000..5e4b271d41d
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/pr82267.c
@@ -0,0 +1,14 @@ 
+/* { dg-do compile { target { ! ia32 } } } */
+/* { dg-require-effective-target maybe_x32 } */
+/* { dg-options "-O2 -mx32 -maddress-mode=short" } */
+
+int
+stackuse (void)
+{
+  volatile int foo = 2;
+  return foo * 3;
+}
+
+/* Verify we that use %rsp to access stack.  */
+/* { dg-final { scan-assembler-not "%esp" } } */
+/* { dg-final { scan-assembler "%rsp" } } */