diff mbox series

i386: Disallow long address mode in the x32 mode. [PR 117418]

Message ID 20241108024052.2847832-1-lin1.hu@intel.com
State New
Headers show
Series i386: Disallow long address mode in the x32 mode. [PR 117418] | expand

Commit Message

Hu, Lin1 Nov. 8, 2024, 2:40 a.m. UTC 
Hi, all

-maddress-mode=long will let Pmode = DI_mode, but -mx32 request x32 ABI.
So raise an error to avoid ICE.

Bootstrapped and regtested, OK for trunk?

BRs,
Lin

gcc/ChangeLog:

	PR target/117418
	* config/i386/i386-options.cc (ix86_option_override_internal): raise an
	error with option -mx32 -maddress-mode=long.

gcc/testsuite/ChangeLog:

	PR target/117418
	* gcc.target/i386/pr117418-1.c: New test.
---
 gcc/config/i386/i386-options.cc            |  4 ++++
 gcc/testsuite/gcc.target/i386/pr117418-1.c | 13 +++++++++++++
 2 files changed, 17 insertions(+)
 create mode 100644 gcc/testsuite/gcc.target/i386/pr117418-1.c

Comments

H.J. Lu Nov. 8, 2024, 4:17 a.m. UTC | #1 
On Fri, Nov 8, 2024 at 10:41 AM Hu, Lin1 <lin1.hu@intel.com> wrote:
>
> Hi, all
>
> -maddress-mode=long will let Pmode = DI_mode, but -mx32 request x32 ABI.
> So raise an error to avoid ICE.
>
> Bootstrapped and regtested, OK for trunk?
>
> BRs,
> Lin
>
> gcc/ChangeLog:
>
>         PR target/117418
>         * config/i386/i386-options.cc (ix86_option_override_internal): raise an
>         error with option -mx32 -maddress-mode=long.
>
> gcc/testsuite/ChangeLog:
>
>         PR target/117418
>         * gcc.target/i386/pr117418-1.c: New test.
> ---
>  gcc/config/i386/i386-options.cc            |  4 ++++
>  gcc/testsuite/gcc.target/i386/pr117418-1.c | 13 +++++++++++++
>  2 files changed, 17 insertions(+)
>  create mode 100644 gcc/testsuite/gcc.target/i386/pr117418-1.c
>
> diff --git a/gcc/config/i386/i386-options.cc b/gcc/config/i386/i386-options.cc
> index 239269ecbdd..ba1abea2537 100644
> --- a/gcc/config/i386/i386-options.cc
> +++ b/gcc/config/i386/i386-options.cc
> @@ -2190,6 +2190,10 @@ ix86_option_override_internal (bool main_args_p,
>         error ("address mode %qs not supported in the %s bit mode",
>                TARGET_64BIT_P (opts->x_ix86_isa_flags) ? "short" : "long",
>                TARGET_64BIT_P (opts->x_ix86_isa_flags) ? "64" : "32");
> +
> +      if (TARGET_X32_P (opts->x_ix86_isa_flags)
> +         && opts_set->x_ix86_pmode == PMODE_DI)
> +       error ("address mode 'long' not supported in the x32 ABI");

This looks wrong.   Try the encoded patch.

>      }
>    else
>      opts->x_ix86_pmode = TARGET_LP64_P (opts->x_ix86_isa_flags)
> diff --git a/gcc/testsuite/gcc.target/i386/pr117418-1.c b/gcc/testsuite/gcc.target/i386/pr117418-1.c
> new file mode 100644
> index 00000000000..08430ef9d4b
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/i386/pr117418-1.c
> @@ -0,0 +1,13 @@
> +/* PR target/117418 */
> +/* { dg-do compile } */
> +/* { dg-options "-maddress-mode=long -mwidekl -mx32" } */
> +/* { dg-error "address mode 'long' not supported in the x32 ABI" "" { target *-*-* } 0 } */
> +
> +typedef __attribute__((__vector_size__(16))) long long V;
> +V a;
> +
> +void
> +foo()
> +{
> +    __builtin_ia32_encodekey256_u32(0, a, a, &a);
> +}
> --
> 2.31.1
>
Hongtao Liu Nov. 8, 2024, 5:21 a.m. UTC | #2 
On Fri, Nov 8, 2024 at 12:18 PM H.J. Lu <hjl.tools@gmail.com> wrote:
>
> On Fri, Nov 8, 2024 at 10:41 AM Hu, Lin1 <lin1.hu@intel.com> wrote:
> >
> > Hi, all
> >
> > -maddress-mode=long will let Pmode = DI_mode, but -mx32 request x32 ABI.
> > So raise an error to avoid ICE.
> >
> > Bootstrapped and regtested, OK for trunk?
> >
> > BRs,
> > Lin
> >
> > gcc/ChangeLog:
> >
> >         PR target/117418
> >         * config/i386/i386-options.cc (ix86_option_override_internal): raise an
> >         error with option -mx32 -maddress-mode=long.
> >
> > gcc/testsuite/ChangeLog:
> >
> >         PR target/117418
> >         * gcc.target/i386/pr117418-1.c: New test.
> > ---
> >  gcc/config/i386/i386-options.cc            |  4 ++++
> >  gcc/testsuite/gcc.target/i386/pr117418-1.c | 13 +++++++++++++
> >  2 files changed, 17 insertions(+)
> >  create mode 100644 gcc/testsuite/gcc.target/i386/pr117418-1.c
> >
> > diff --git a/gcc/config/i386/i386-options.cc b/gcc/config/i386/i386-options.cc
> > index 239269ecbdd..ba1abea2537 100644
> > --- a/gcc/config/i386/i386-options.cc
> > +++ b/gcc/config/i386/i386-options.cc
> > @@ -2190,6 +2190,10 @@ ix86_option_override_internal (bool main_args_p,
> >         error ("address mode %qs not supported in the %s bit mode",
> >                TARGET_64BIT_P (opts->x_ix86_isa_flags) ? "short" : "long",
> >                TARGET_64BIT_P (opts->x_ix86_isa_flags) ? "64" : "32");
> > +
> > +      if (TARGET_X32_P (opts->x_ix86_isa_flags)
> > +         && opts_set->x_ix86_pmode == PMODE_DI)
> > +       error ("address mode 'long' not supported in the x32 ABI");
>
> This looks wrong.   Try the encoded patch.
>
So it means -maddress-mode=long will override x32 to use 64-bit pointer?
> >      }
> >    else
> >      opts->x_ix86_pmode = TARGET_LP64_P (opts->x_ix86_isa_flags)
> > diff --git a/gcc/testsuite/gcc.target/i386/pr117418-1.c b/gcc/testsuite/gcc.target/i386/pr117418-1.c
> > new file mode 100644
> > index 00000000000..08430ef9d4b
> > --- /dev/null
> > +++ b/gcc/testsuite/gcc.target/i386/pr117418-1.c
> > @@ -0,0 +1,13 @@
> > +/* PR target/117418 */
> > +/* { dg-do compile } */
> > +/* { dg-options "-maddress-mode=long -mwidekl -mx32" } */
> > +/* { dg-error "address mode 'long' not supported in the x32 ABI" "" { target *-*-* } 0 } */
> > +
> > +typedef __attribute__((__vector_size__(16))) long long V;
> > +V a;
> > +
> > +void
> > +foo()
> > +{
> > +    __builtin_ia32_encodekey256_u32(0, a, a, &a);
> > +}
> > --
> > 2.31.1
> >
>
>
> --
> H.J.
Hongtao Liu Nov. 8, 2024, 5:52 a.m. UTC | #3 
On Fri, Nov 8, 2024 at 1:21 PM Hongtao Liu <crazylht@gmail.com> wrote:
>
> On Fri, Nov 8, 2024 at 12:18 PM H.J. Lu <hjl.tools@gmail.com> wrote:
> >
> > On Fri, Nov 8, 2024 at 10:41 AM Hu, Lin1 <lin1.hu@intel.com> wrote:
> > >
> > > Hi, all
> > >
> > > -maddress-mode=long will let Pmode = DI_mode, but -mx32 request x32 ABI.
> > > So raise an error to avoid ICE.
> > >
> > > Bootstrapped and regtested, OK for trunk?
> > >
> > > BRs,
> > > Lin
> > >
> > > gcc/ChangeLog:
> > >
> > >         PR target/117418
> > >         * config/i386/i386-options.cc (ix86_option_override_internal): raise an
> > >         error with option -mx32 -maddress-mode=long.
> > >
> > > gcc/testsuite/ChangeLog:
> > >
> > >         PR target/117418
> > >         * gcc.target/i386/pr117418-1.c: New test.
> > > ---
> > >  gcc/config/i386/i386-options.cc            |  4 ++++
> > >  gcc/testsuite/gcc.target/i386/pr117418-1.c | 13 +++++++++++++
> > >  2 files changed, 17 insertions(+)
> > >  create mode 100644 gcc/testsuite/gcc.target/i386/pr117418-1.c
> > >
> > > diff --git a/gcc/config/i386/i386-options.cc b/gcc/config/i386/i386-options.cc
> > > index 239269ecbdd..ba1abea2537 100644
> > > --- a/gcc/config/i386/i386-options.cc
> > > +++ b/gcc/config/i386/i386-options.cc
> > > @@ -2190,6 +2190,10 @@ ix86_option_override_internal (bool main_args_p,
> > >         error ("address mode %qs not supported in the %s bit mode",
> > >                TARGET_64BIT_P (opts->x_ix86_isa_flags) ? "short" : "long",
> > >                TARGET_64BIT_P (opts->x_ix86_isa_flags) ? "64" : "32");
> > > +
> > > +      if (TARGET_X32_P (opts->x_ix86_isa_flags)
> > > +         && opts_set->x_ix86_pmode == PMODE_DI)
> > > +       error ("address mode 'long' not supported in the x32 ABI");
> >
> > This looks wrong.   Try the encoded patch.
> >
> So it means -maddress-mode=long will override x32 to use 64-bit pointer?
No, answered by myself.
The upper 32-bit is zero, so it's still 32-bit memory space although
it uses a 64-bit register as a pointer.
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=82267
> > >      }
> > >    else
> > >      opts->x_ix86_pmode = TARGET_LP64_P (opts->x_ix86_isa_flags)
> > > diff --git a/gcc/testsuite/gcc.target/i386/pr117418-1.c b/gcc/testsuite/gcc.target/i386/pr117418-1.c
> > > new file mode 100644
> > > index 00000000000..08430ef9d4b
> > > --- /dev/null
> > > +++ b/gcc/testsuite/gcc.target/i386/pr117418-1.c
> > > @@ -0,0 +1,13 @@
> > > +/* PR target/117418 */
> > > +/* { dg-do compile } */
> > > +/* { dg-options "-maddress-mode=long -mwidekl -mx32" } */
> > > +/* { dg-error "address mode 'long' not supported in the x32 ABI" "" { target *-*-* } 0 } */
> > > +
> > > +typedef __attribute__((__vector_size__(16))) long long V;
> > > +V a;
> > > +
> > > +void
> > > +foo()
> > > +{
> > > +    __builtin_ia32_encodekey256_u32(0, a, a, &a);
> > > +}
> > > --
> > > 2.31.1
> > >
> >
> >
> > --
> > H.J.
>
>
>
> --
> BR,
> Hongtao
Uros Bizjak Nov. 8, 2024, 7:18 a.m. UTC | #4 
On Fri, Nov 8, 2024 at 6:52 AM Hongtao Liu <crazylht@gmail.com> wrote:

> > > >         PR target/117418
> > > >         * config/i386/i386-options.cc (ix86_option_override_internal): raise an
> > > >         error with option -mx32 -maddress-mode=long.
> > > >
> > > > gcc/testsuite/ChangeLog:
> > > >
> > > >         PR target/117418
> > > >         * gcc.target/i386/pr117418-1.c: New test.
> > > > ---
> > > >  gcc/config/i386/i386-options.cc            |  4 ++++
> > > >  gcc/testsuite/gcc.target/i386/pr117418-1.c | 13 +++++++++++++
> > > >  2 files changed, 17 insertions(+)
> > > >  create mode 100644 gcc/testsuite/gcc.target/i386/pr117418-1.c
> > > >
> > > > diff --git a/gcc/config/i386/i386-options.cc b/gcc/config/i386/i386-options.cc
> > > > index 239269ecbdd..ba1abea2537 100644
> > > > --- a/gcc/config/i386/i386-options.cc
> > > > +++ b/gcc/config/i386/i386-options.cc
> > > > @@ -2190,6 +2190,10 @@ ix86_option_override_internal (bool main_args_p,
> > > >         error ("address mode %qs not supported in the %s bit mode",
> > > >                TARGET_64BIT_P (opts->x_ix86_isa_flags) ? "short" : "long",
> > > >                TARGET_64BIT_P (opts->x_ix86_isa_flags) ? "64" : "32");
> > > > +
> > > > +      if (TARGET_X32_P (opts->x_ix86_isa_flags)
> > > > +         && opts_set->x_ix86_pmode == PMODE_DI)
> > > > +       error ("address mode 'long' not supported in the x32 ABI");
> > >
> > > This looks wrong.   Try the encoded patch.
> > >
> > So it means -maddress-mode=long will override x32 to use 64-bit pointer?
> No, answered by myself.
> The upper 32-bit is zero, so it's still 32-bit memory space although
> it uses a 64-bit register as a pointer.
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=82267

Yes, ptr_mode and Pmode are two different things. ptr_mode is ABI
mandated pointer mode, while Pmode is an implementation detail. As
most of the targets, x32 used Pmode == ptr_mode, but as evident from
the quoted PR, it resulted in many 0x67 prefixes due to address size
overrides to handle SImode ptr_mode ABI requirements.

-maddress-mode=long was added just for x32 to mitigate this problem.
It was beneficial for some applications, thus the request to make it
the default, but it remained as it is.

Because this option is not the default one, it has (some) tendency to
bitrot, when some new committed code assumes ptr_mode == Pmode. IIRC,
HJ had a tester that exercised -maddress-mode=long for -mx32 to hunt
middle-end and back-end issues with ptr_mode != Pmode.

Uros.
Hongtao Liu Nov. 8, 2024, 8:12 a.m. UTC | #5 
On Fri, Nov 8, 2024 at 3:18 PM Uros Bizjak <ubizjak@gmail.com> wrote:
>
> On Fri, Nov 8, 2024 at 6:52 AM Hongtao Liu <crazylht@gmail.com> wrote:
>
> > > > >         PR target/117418
> > > > >         * config/i386/i386-options.cc (ix86_option_override_internal): raise an
> > > > >         error with option -mx32 -maddress-mode=long.
> > > > >
> > > > > gcc/testsuite/ChangeLog:
> > > > >
> > > > >         PR target/117418
> > > > >         * gcc.target/i386/pr117418-1.c: New test.
> > > > > ---
> > > > >  gcc/config/i386/i386-options.cc            |  4 ++++
> > > > >  gcc/testsuite/gcc.target/i386/pr117418-1.c | 13 +++++++++++++
> > > > >  2 files changed, 17 insertions(+)
> > > > >  create mode 100644 gcc/testsuite/gcc.target/i386/pr117418-1.c
> > > > >
> > > > > diff --git a/gcc/config/i386/i386-options.cc b/gcc/config/i386/i386-options.cc
> > > > > index 239269ecbdd..ba1abea2537 100644
> > > > > --- a/gcc/config/i386/i386-options.cc
> > > > > +++ b/gcc/config/i386/i386-options.cc
> > > > > @@ -2190,6 +2190,10 @@ ix86_option_override_internal (bool main_args_p,
> > > > >         error ("address mode %qs not supported in the %s bit mode",
> > > > >                TARGET_64BIT_P (opts->x_ix86_isa_flags) ? "short" : "long",
> > > > >                TARGET_64BIT_P (opts->x_ix86_isa_flags) ? "64" : "32");
> > > > > +
> > > > > +      if (TARGET_X32_P (opts->x_ix86_isa_flags)
> > > > > +         && opts_set->x_ix86_pmode == PMODE_DI)
> > > > > +       error ("address mode 'long' not supported in the x32 ABI");
> > > >
> > > > This looks wrong.   Try the encoded patch.
> > > >
> > > So it means -maddress-mode=long will override x32 to use 64-bit pointer?
> > No, answered by myself.
> > The upper 32-bit is zero, so it's still 32-bit memory space although
> > it uses a 64-bit register as a pointer.
> > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=82267
>
> Yes, ptr_mode and Pmode are two different things. ptr_mode is ABI
> mandated pointer mode, while Pmode is an implementation detail. As
> most of the targets, x32 used Pmode == ptr_mode, but as evident from
> the quoted PR, it resulted in many 0x67 prefixes due to address size
> overrides to handle SImode ptr_mode ABI requirements.
>
> -maddress-mode=long was added just for x32 to mitigate this problem.
> It was beneficial for some applications, thus the request to make it
> the default, but it remained as it is.
>
> Because this option is not the default one, it has (some) tendency to
> bitrot, when some new committed code assumes ptr_mode == Pmode. IIRC,
> HJ had a tester that exercised -maddress-mode=long for -mx32 to hunt
> middle-end and back-end issues with ptr_mode != Pmode.
Thanks for explaining the background and reasons for it
>
> Uros.
diff mbox series

Patch

diff --git a/gcc/config/i386/i386-options.cc b/gcc/config/i386/i386-options.cc
index 239269ecbdd..ba1abea2537 100644
--- a/gcc/config/i386/i386-options.cc
+++ b/gcc/config/i386/i386-options.cc
@@ -2190,6 +2190,10 @@  ix86_option_override_internal (bool main_args_p,
 	error ("address mode %qs not supported in the %s bit mode",
 	       TARGET_64BIT_P (opts->x_ix86_isa_flags) ? "short" : "long",
 	       TARGET_64BIT_P (opts->x_ix86_isa_flags) ? "64" : "32");
+
+      if (TARGET_X32_P (opts->x_ix86_isa_flags)
+	  && opts_set->x_ix86_pmode == PMODE_DI)
+	error ("address mode 'long' not supported in the x32 ABI");
     }
   else
     opts->x_ix86_pmode = TARGET_LP64_P (opts->x_ix86_isa_flags)
diff --git a/gcc/testsuite/gcc.target/i386/pr117418-1.c b/gcc/testsuite/gcc.target/i386/pr117418-1.c
new file mode 100644
index 00000000000..08430ef9d4b
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/pr117418-1.c
@@ -0,0 +1,13 @@ 
+/* PR target/117418 */
+/* { dg-do compile } */
+/* { dg-options "-maddress-mode=long -mwidekl -mx32" } */
+/* { dg-error "address mode 'long' not supported in the x32 ABI" "" { target *-*-* } 0 } */
+
+typedef __attribute__((__vector_size__(16))) long long V;
+V a;
+
+void
+foo()
+{
+    __builtin_ia32_encodekey256_u32(0, a, a, &a); 
+}