Message ID | 20210829152410.90290-1-hjl.tools@gmail.com |
---|---|
State | New |
Headers | show |
Series | x86-64: Add ABI warning for 64-bit vectors | expand |
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
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 >
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 --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"