diff mbox series

x86-64: Add ABI warning for 64-bit vectors

Message ID 20210829152410.90290-1-hjl.tools@gmail.com
State New
Headers show
Series x86-64: Add ABI warning for 64-bit vectors | expand

Commit Message

H.J. Lu Aug. 29, 2021, 3:24 p.m. UTC
TYPE_MODE of record and union depends on whether vector_mode_supported_p
returns true or not. x86-64 backend uses TYPE_MODE to decide how to pass
a parameter and return a value in a function.  64-bit integer vectors
were supported only by MMX and 64-bit float vector was supported only by
3DNOW.  GCC 10 enabled 64-bit integer vectors without MMX by:

commit dfa61b9ed06d71901c4c430caa89820972ad68fe
Author: H.J. Lu <hongjiu.lu@intel.com>
Date:   Wed May 15 15:02:54 2019 +0000

    i386: Allow MMX register modes in SSE registers

    In 64-bit mode, SSE2 can be used to emulate MMX instructions without
    3DNOW.  We can use SSE2 to support MMX register modes.

GCC 11 enabled 64-bit float vector without 3DNOW by:

commit 7c355156aa20eaec7401d7c66f6a6cfbe597abc2
Author: Uros Bizjak <ubizjak@gmail.com>
Date:   Mon May 11 11:16:31 2020 +0200

    i386: Vectorize basic V2SFmode operations [PR94913]

    Enable V2SFmode vectorization and vectorize V2SFmode PLUS,
    MINUS, MULT, MIN and MAX operations using XMM registers.

Add ABI warnings for 64-bit integer vectors without MMX and 64-bit float
vector without 3DNOW.

gcc/

	PR target/102027
	PR target/102105
	* config/i386/i386.c (single_m64_base_type): New function.
	(type_natural_mode): Add ABI warnings for 64-bit vectors.

gcc/testsuite/

	PR target/102027
	PR target/102105
	* gcc.target/i386/pr102027-1.c: New test.
	* gcc.target/i386/pr102027-2.c: Likewise.
	* gcc.target/i386/pr102027-3.c: Likewise.
	* gcc.target/i386/pr102105-1.c: Likewise.
	* gcc.target/i386/pr102105-2.c: Likewise.
	* gcc.target/i386/pr102105-3.c: Likewise.
	* gcc.target/i386/pr102105-4.c: Likewise.
	* gcc.target/i386/sse2-mmx-4.c: Add -Wno-psabi.
---
 gcc/config/i386/i386.c                     | 98 ++++++++++++++++++++++
 gcc/testsuite/gcc.target/i386/pr102027-1.c | 15 ++++
 gcc/testsuite/gcc.target/i386/pr102027-2.c | 15 ++++
 gcc/testsuite/gcc.target/i386/pr102027-3.c | 17 ++++
 gcc/testsuite/gcc.target/i386/pr102105-1.c | 15 ++++
 gcc/testsuite/gcc.target/i386/pr102105-2.c | 15 ++++
 gcc/testsuite/gcc.target/i386/pr102105-3.c | 17 ++++
 gcc/testsuite/gcc.target/i386/pr102105-4.c | 17 ++++
 gcc/testsuite/gcc.target/i386/sse2-mmx-4.c |  2 +-
 9 files changed, 210 insertions(+), 1 deletion(-)
 create mode 100644 gcc/testsuite/gcc.target/i386/pr102027-1.c
 create mode 100644 gcc/testsuite/gcc.target/i386/pr102027-2.c
 create mode 100644 gcc/testsuite/gcc.target/i386/pr102027-3.c
 create mode 100644 gcc/testsuite/gcc.target/i386/pr102105-1.c
 create mode 100644 gcc/testsuite/gcc.target/i386/pr102105-2.c
 create mode 100644 gcc/testsuite/gcc.target/i386/pr102105-3.c
 create mode 100644 gcc/testsuite/gcc.target/i386/pr102105-4.c

Comments

Jakub Jelinek Aug. 29, 2021, 3:34 p.m. UTC | #1
On Sun, Aug 29, 2021 at 08:24:10AM -0700, H.J. Lu via Gcc-patches wrote:
> +      if (gcc_version)
> +	{
> +	  if (in_return)
> +	    {
> +	      static bool warnedm64_ret;
> +	      if (!warnedm64_ret)
> +		{
> +		  if (warning (OPT_Wpsabi,
> +			       "the ABI of returning structure with a "
> +			       "64-bit vector has changed in GCC %d.1",
> +			       gcc_version))
> +		    warnedm64_ret = true;
> +		}
> +	    }
> +	  else
> +	    {
> +	      static bool warnedm64;
> +	      if (!warnedm64)
> +		{
> +		  if (warning (OPT_Wpsabi,
> +			       "the ABI of passing structure with a "
> +			       "64-bit vector has changed in GCC %d.1",
> +			       gcc_version))
> +		    warnedm64 = true;
> +		}
> +	    }
> +	}

Other -Wpsabi diagnostics in i386.c seems to be done using inform rather
than warning, why the change?
And, can you add wwwdocs description of the ABI change and use
in %{GCC %d.1%}, so that on recent terminals people can find out the
details by clicking on it?
See
          if (!warned && warn_psabi)
            {
              const char *url
                = CHANGES_ROOT_URL "gcc-11/changes.html#ia32_atomic";
    
              warned = true;
              inform (input_location, "the alignment of %<_Atomic %T%> "
                                      "fields changed in %{GCC 11.1%}",
                      TYPE_MAIN_VARIANT (type), url);
            }

	Jakub
H.J. Lu Aug. 29, 2021, 4:17 p.m. UTC | #2
On Sun, Aug 29, 2021 at 8:34 AM Jakub Jelinek <jakub@redhat.com> wrote:
>
> On Sun, Aug 29, 2021 at 08:24:10AM -0700, H.J. Lu via Gcc-patches wrote:
> > +      if (gcc_version)
> > +     {
> > +       if (in_return)
> > +         {
> > +           static bool warnedm64_ret;
> > +           if (!warnedm64_ret)
> > +             {
> > +               if (warning (OPT_Wpsabi,
> > +                            "the ABI of returning structure with a "
> > +                            "64-bit vector has changed in GCC %d.1",
> > +                            gcc_version))
> > +                 warnedm64_ret = true;
> > +             }
> > +         }
> > +       else
> > +         {
> > +           static bool warnedm64;
> > +           if (!warnedm64)
> > +             {
> > +               if (warning (OPT_Wpsabi,
> > +                            "the ABI of passing structure with a "
> > +                            "64-bit vector has changed in GCC %d.1",
> > +                            gcc_version))
> > +                 warnedm64 = true;
> > +             }
> > +         }
> > +     }
>
> Other -Wpsabi diagnostics in i386.c seems to be done using inform rather
> than warning, why the change?

I will fix it.

> And, can you add wwwdocs description of the ABI change and use
> in %{GCC %d.1%}, so that on recent terminals people can find out the
> details by clicking on it?

How does it work?

[hjl@gnu-tgl-2 gcc]$ gcc -S -m32
/export/gnu/import/git/gitlab/x86-gcc/gcc/testsuite/gcc.target/i386/pr65146.c
/export/gnu/import/git/gitlab/x86-gcc/gcc/testsuite/gcc.target/i386/pr65146.c:5:8:
note: the alignment of ‘_Atomic long long int’ fields changed in GCC
11.1
    5 | struct A { char a; _Atomic long long b; };
      |        ^
[hjl@gnu-tgl-2 gcc]$ gcc -v
Using built-in specs.
COLLECT_GCC=gcc
COLLECT_LTO_WRAPPER=/usr/libexec/gcc/x86_64-redhat-linux/11/lto-wrapper
OFFLOAD_TARGET_NAMES=nvptx-none
OFFLOAD_TARGET_DEFAULT=1
Target: x86_64-redhat-linux
Configured with: ../configure --enable-bootstrap
--enable-languages=c,c++,fortran,objc,obj-c++,ada,go,d,lto
--prefix=/usr --mandir=/usr/share/man --infodir=/usr/share/info
--with-bugurl=http://bugzilla.redhat.com/bugzilla --enable-shared
--enable-threads=posix --enable-checking=release --enable-multilib
--with-system-zlib --enable-__cxa_atexit
--disable-libunwind-exceptions --enable-gnu-unique-object
--enable-linker-build-id --with-gcc-major-version-only
--with-linker-hash-style=gnu --enable-plugin --enable-initfini-array
--with-isl=/builddir/build/BUILD/gcc-11.2.1-20210728/obj-x86_64-redhat-linux/isl-install
--enable-offload-targets=nvptx-none --without-cuda-driver
--enable-gnu-indirect-function --enable-cet --with-tune=generic
--with-arch_32=i686 --with-multilib-list=m32,m64,mx32
--build=x86_64-redhat-linux
Thread model: posix
Supported LTO compression algorithms: zlib zstd
gcc version 11.2.1 20210728 (Red Hat 11.2.1-1) (GCC)
[hjl@gnu-tgl-2 gcc]$

There is no URL to click.

> See
>           if (!warned && warn_psabi)
>             {
>               const char *url
>                 = CHANGES_ROOT_URL "gcc-11/changes.html#ia32_atomic";
>
>               warned = true;
>               inform (input_location, "the alignment of %<_Atomic %T%> "
>                                       "fields changed in %{GCC 11.1%}",
>                       TYPE_MAIN_VARIANT (type), url);
>             }
>
>         Jakub
>
Jakub Jelinek Aug. 29, 2021, 6:56 p.m. UTC | #3
On Sun, Aug 29, 2021 at 09:17:10AM -0700, H.J. Lu wrote:
> How does it work?

Depends on the terminal.  E.g. in recent, at most a few years old
gnome-terminal, that GCC 11.1 is dotted underlined and hovering with mouse
on it shows it normal underlined and prints the URL, right click shows a
menu in which one can Open Hyperlink.
See https://gist.github.com/egmontkob/eb114294efbcd5adb1944c9f3cb5feda

	Jakub
diff mbox series

Patch

diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index 3bb2cab57a3..5d9cad7468b 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -1840,6 +1840,54 @@  init_cumulative_args (CUMULATIVE_ARGS *cum,  /* Argument info to initialize */
   cfun->machine->arg_reg_available = (cum->nregs > 0);
 }
 
+/* Return the single 64-bit vector type of TYPE.  */
+
+static const_tree
+single_m64_base_type (const_tree type)
+{
+  if ((TREE_CODE (type) == RECORD_TYPE
+       || TREE_CODE (type) == UNION_TYPE
+       || TREE_CODE (type) == QUAL_UNION_TYPE)
+      && int_size_in_bytes (type) == 8)
+    {
+      const_tree field;
+      const_tree first_field = nullptr;
+
+      for (field = TYPE_FIELDS (type);
+	   field;
+	   field = DECL_CHAIN (field))
+	if (TREE_CODE (field) == FIELD_DECL)
+	  {
+	    if (TREE_TYPE (field) == error_mark_node)
+	      continue;
+
+	    /* Skip if structure has more than one field.  */
+	    if (first_field)
+	      return nullptr;
+
+	    first_field = field;
+	  }
+
+      /* Skip if structure doesn't have any fields.  */
+      if (!first_field)
+	return nullptr;
+
+      type = TREE_TYPE (first_field);
+
+      /* Skip if structure size isn't 64 bits.  */
+      if (int_size_in_bytes (type) != 8)
+	return nullptr;
+
+      /* Return if a 64-bit vector is found.  */
+      if (TREE_CODE (type) == VECTOR_TYPE)
+	return type;
+
+      return single_m64_base_type (type);
+    }
+
+  return nullptr;
+}
+
 /* Return the "natural" mode for TYPE.  In most cases, this is just TYPE_MODE.
    But in the case of vector types, it is some vector mode.
 
@@ -1863,6 +1911,56 @@  type_natural_mode (const_tree type, const CUMULATIVE_ARGS *cum,
 {
   machine_mode mode = TYPE_MODE (type);
 
+  const_tree m64_type = single_m64_base_type (type);
+  if (m64_type)
+    {
+      mode = TYPE_MODE (m64_type);
+
+      int gcc_version = 0;
+      if (mode == V2SFmode)
+	{
+	  /* GCC 11 enables V2SFmode without TARGET_3DNOW.  */
+	  if (!TARGET_3DNOW)
+	    gcc_version = 11;
+	}
+      else
+	{
+	  /* GCC 10 enables other MMX modes without TARGET_MMX.  */
+	  if (!TARGET_MMX)
+	    gcc_version = 10;
+	}
+
+      if (gcc_version)
+	{
+	  if (in_return)
+	    {
+	      static bool warnedm64_ret;
+	      if (!warnedm64_ret)
+		{
+		  if (warning (OPT_Wpsabi,
+			       "the ABI of returning structure with a "
+			       "64-bit vector has changed in GCC %d.1",
+			       gcc_version))
+		    warnedm64_ret = true;
+		}
+	    }
+	  else
+	    {
+	      static bool warnedm64;
+	      if (!warnedm64)
+		{
+		  if (warning (OPT_Wpsabi,
+			       "the ABI of passing structure with a "
+			       "64-bit vector has changed in GCC %d.1",
+			       gcc_version))
+		    warnedm64 = true;
+		}
+	    }
+	}
+
+      return mode;
+    }
+
   if (TREE_CODE (type) == VECTOR_TYPE && !VECTOR_MODE_P (mode))
     {
       HOST_WIDE_INT size = int_size_in_bytes (type);
diff --git a/gcc/testsuite/gcc.target/i386/pr102027-1.c b/gcc/testsuite/gcc.target/i386/pr102027-1.c
new file mode 100644
index 00000000000..078ad8055e0
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/pr102027-1.c
@@ -0,0 +1,15 @@ 
+/* { dg-do compile { target { ! ia32 } } } */
+/* { dg-options "-mno-3dnow" } */
+
+struct V2SF {
+  __attribute__((__vector_size__(2 * sizeof(float)))) float v;
+};
+extern struct V2SF x;
+extern void foo (struct V2SF);
+
+struct V2SF
+bar (void)
+{ /* { dg-warning "the ABI of returning structure with a 64-bit vector has changed in GCC 11.1" } */
+  foo (x); /* { dg-warning "the ABI of passing structure with a 64-bit vector has changed in GCC 11.1" } */
+  return x;
+}
diff --git a/gcc/testsuite/gcc.target/i386/pr102027-2.c b/gcc/testsuite/gcc.target/i386/pr102027-2.c
new file mode 100644
index 00000000000..ed48c238665
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/pr102027-2.c
@@ -0,0 +1,15 @@ 
+/* { dg-do compile { target { ! ia32 } } } */
+/* { dg-options "-mno-3dnow" } */
+
+union U2SF {
+  __attribute__((__vector_size__(2 * sizeof(float)))) float v;
+};
+extern union U2SF x;
+extern void foo (union U2SF);
+
+union U2SF
+bar (void)
+{ /* { dg-warning "the ABI of returning structure with a 64-bit vector has changed in GCC 11.1" } */
+  foo (x); /* { dg-warning "the ABI of passing structure with a 64-bit vector has changed in GCC 11.1" } */
+  return x;
+}
diff --git a/gcc/testsuite/gcc.target/i386/pr102027-3.c b/gcc/testsuite/gcc.target/i386/pr102027-3.c
new file mode 100644
index 00000000000..e41b8b73f98
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/pr102027-3.c
@@ -0,0 +1,17 @@ 
+/* { dg-do compile { target { ! ia32 } } } */
+/* { dg-options "-mno-3dnow" } */
+
+struct V2SF1 {
+  __attribute__((__vector_size__(2 * sizeof(float)))) float v;
+};
+struct V2SF {
+  struct V2SF1 x;
+};
+extern struct V2SF x;
+extern void foo (struct V2SF);
+
+void
+bar (void)
+{
+  foo (x); /* { dg-warning "the ABI of passing structure with a 64-bit vector has changed in GCC 11.1" } */
+}
diff --git a/gcc/testsuite/gcc.target/i386/pr102105-1.c b/gcc/testsuite/gcc.target/i386/pr102105-1.c
new file mode 100644
index 00000000000..ec151a6e826
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/pr102105-1.c
@@ -0,0 +1,15 @@ 
+/* { dg-do compile { target { ! ia32 } } } */
+/* { dg-options "-mno-mmx" } */
+
+struct V8QI {
+  __attribute__((__vector_size__(8))) char v;
+};
+extern struct V8QI x;
+extern void foo (struct V8QI);
+
+struct V8QI
+bar (void)
+{ /* { dg-warning "the ABI of returning structure with a 64-bit vector has changed in GCC 10.1" } */
+  foo (x); /* { dg-warning "the ABI of passing structure with a 64-bit vector has changed in GCC 10.1" } */
+  return x;
+}
diff --git a/gcc/testsuite/gcc.target/i386/pr102105-2.c b/gcc/testsuite/gcc.target/i386/pr102105-2.c
new file mode 100644
index 00000000000..47dc7e37791
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/pr102105-2.c
@@ -0,0 +1,15 @@ 
+/* { dg-do compile { target { ! ia32 } } } */
+/* { dg-options "-mno-mmx" } */
+
+union U8QI {
+  __attribute__((__vector_size__(8))) char v;
+};
+extern union U8QI x;
+extern void foo (union U8QI);
+
+union U8QI
+bar (void)
+{ /* { dg-warning "the ABI of returning structure with a 64-bit vector has changed in GCC 10.1" } */
+  foo (x); /* { dg-warning "the ABI of passing structure with a 64-bit vector has changed in GCC 10.1" } */
+  return x;
+}
diff --git a/gcc/testsuite/gcc.target/i386/pr102105-3.c b/gcc/testsuite/gcc.target/i386/pr102105-3.c
new file mode 100644
index 00000000000..45bb4ba493a
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/pr102105-3.c
@@ -0,0 +1,17 @@ 
+/* { dg-do compile { target { ! ia32 } } } */
+/* { dg-options "-mno-mmx" } */
+
+struct V8QI1 {
+  __attribute__((__vector_size__(8))) char v;
+};
+struct V8QI {
+  struct V8QI1 x;
+};
+extern struct V8QI x;
+extern void foo (struct V8QI);
+
+void
+bar (void)
+{
+  foo (x); /* { dg-warning "the ABI of passing structure with a 64-bit vector has changed in GCC 10.1" } */
+}
diff --git a/gcc/testsuite/gcc.target/i386/pr102105-4.c b/gcc/testsuite/gcc.target/i386/pr102105-4.c
new file mode 100644
index 00000000000..763d51ccf0b
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/pr102105-4.c
@@ -0,0 +1,17 @@ 
+/* { dg-do compile { target { ! ia32 } } } */
+/* { dg-options "-mno-mmx" } */
+
+struct v1di
+{
+  __attribute__((__vector_size__(8))) long long ll;
+};
+extern struct v1di x;
+
+extern void foo (struct v1di);
+
+struct v1di
+bar (void)
+{ /* { dg-warning "the ABI of returning structure with a 64-bit vector has changed in GCC 10.1" } */
+  foo (x); /* { dg-warning "the ABI of passing structure with a 64-bit vector has changed in GCC 10.1" } */
+  return x;
+}
diff --git a/gcc/testsuite/gcc.target/i386/sse2-mmx-4.c b/gcc/testsuite/gcc.target/i386/sse2-mmx-4.c
index d923724fc1c..01ee8db4a4a 100644
--- a/gcc/testsuite/gcc.target/i386/sse2-mmx-4.c
+++ b/gcc/testsuite/gcc.target/i386/sse2-mmx-4.c
@@ -1,4 +1,4 @@ 
 /* { dg-do run { target { ! ia32 } } } */
-/* { dg-options "-O2 -msse2 -mno-mmx" } */
+/* { dg-options "-O2 -msse2 -mno-mmx -Wno-psabi" } */
 
 #include "mmx-4.c"