On Wed, Jun 24, 2015 at 8:57 AM, Andreas Krebbel
<kreb...@linux.vnet.ibm.com> wrote:
> With this patch .gnu_attribute is used to mark binaries with a vector
> ABI tag. This is required since the z13 vector support breaks the ABI
> of existing vector_size attribute generated vector types:
>
> 1. vector_size(16) and bigger vectors are aligned to 8 byte
> boundaries (formerly vectors were always naturally aligned)
>
> 2. vector_size(16) or smaller vectors are passed via VR if available
> or by value on the stack (formerly vector were passed on the stack by
> reference).
>
> The .gnu_attribute will be used by ld to emit a warning if binaries
> with incompatible ABIs are being linked together:
> https://sourceware.org/ml/binutils/2015-04/msg00316.html
>
> And it will be used by GDB to perform inferior function calls using a
> vector ABI which fits to the binary being debugged:
> https://sourceware.org/ml/gdb-patches/2015-04/msg00833.html
>
> The current implementation tries to only set the attribute if the
> vector types are really used in ABI relevant contexts in order to
> avoid false positives during linking.
>
> However, this unfortunately has some limitations like in the following
> case where an ABI relevant context cannot be detected properly:
>
> typedef int __attribute__((vector_size(16))) v4si;
> struct A
> {
> char x;
> v4si y;
> };
> char a[sizeof(struct A)];
>
> The number of elements in a depends on the ABI (24 with -mvx and 32
> with -mno-vx). However, the implementation is not able to detect this
> since the struct type is not used anywhere else and consequently does
> not survive until the checking code is able to see it.
>
> Ideas about how to improve the implementation without creating too
> many false postives are welcome.
I'd be more conservative and instead hook into
targetm.vector_mode_supported_p (and thus vector_type_mode).
Yes, it will trip on "local" vector types. But I can't see how you
can avoid this in general without seeing the whole program.
If I'd do it retro-actively I'd reverse the flag and instead mark units
which use generic non-z13 vectors...
Note that other targets simply emit -Wpsabi warnings here:
> gcc t.c -S -m32
t.c: In function ‘foo’:
t.c:4:1: warning: SSE vector return without SSE enabled changes the
ABI [-Wpsabi]
{
^
t.c:3:6: note: The ABI for passing parameters with 16-byte alignment
has changed in GCC 4.6
v4si foo (v4si x)
^
t.c:3:6: warning: SSE vector argument without SSE enabled changes the
ABI [-Wpsabi]
for
typedef int v4si __attribute__((vector_size(16)));
v4si foo (v4si x)
{
return x;
}
on i?86 without -msse2. So you could as well do that - warn for vector
type uses on non-z13 and be done with that.
Richard.
> In particular we do not want to set the attribute for local uses of
> vector types as they would be natural for ifunc optimizations.
>
> gcc/
> * config/s390/s390.c (s390_vector_abi): New variable definition.
> (s390_check_type_for_vector_abi): New function.
> (TARGET_ASM_FILE_END): New macro definition.
> (s390_asm_file_end): New function.
> (s390_function_arg): Call s390_check_type_for_vector_abi.
> (s390_gimplify_va_arg): Likewise.
> * configure: Regenerate.
> * configure.ac: Check for .gnu_attribute Binutils feature.
>
> gcc/testsuite/
> * gcc.target/s390/vector/vec-abi-1.c: Add gnu attribute check.
> * gcc.target/s390/vector/vec-abi-attr-1.c: New test.
> * gcc.target/s390/vector/vec-abi-attr-2.c: New test.
> * gcc.target/s390/vector/vec-abi-attr-3.c: New test.
> * gcc.target/s390/vector/vec-abi-attr-4.c: New test.
> * gcc.target/s390/vector/vec-abi-attr-5.c: New test.
> * gcc.target/s390/vector/vec-abi-attr-6.c: New test.
> ---
> gcc/config/s390/s390.c | 121
> ++++++++++++++++++++
> gcc/configure | 36 ++++++
> gcc/configure.ac | 7 ++
> gcc/testsuite/gcc.target/s390/vector/vec-abi-1.c | 1 +
> .../gcc.target/s390/vector/vec-abi-attr-1.c | 18 +++
> .../gcc.target/s390/vector/vec-abi-attr-2.c | 53 +++++++++
> .../gcc.target/s390/vector/vec-abi-attr-3.c | 18 +++
> .../gcc.target/s390/vector/vec-abi-attr-4.c | 17 +++
> .../gcc.target/s390/vector/vec-abi-attr-5.c | 19 +++
> .../gcc.target/s390/vector/vec-abi-attr-6.c | 24 ++++
> 10 files changed, 314 insertions(+)
> create mode 100644 gcc/testsuite/gcc.target/s390/vector/vec-abi-attr-1.c
> create mode 100644 gcc/testsuite/gcc.target/s390/vector/vec-abi-attr-2.c
> create mode 100644 gcc/testsuite/gcc.target/s390/vector/vec-abi-attr-3.c
> create mode 100644 gcc/testsuite/gcc.target/s390/vector/vec-abi-attr-4.c
> create mode 100644 gcc/testsuite/gcc.target/s390/vector/vec-abi-attr-5.c
> create mode 100644 gcc/testsuite/gcc.target/s390/vector/vec-abi-attr-6.c
>
> diff --git a/gcc/config/s390/s390.c b/gcc/config/s390/s390.c
> index d6ed179..934f7c0 100644
> --- a/gcc/config/s390/s390.c
> +++ b/gcc/config/s390/s390.c
> @@ -461,6 +461,97 @@ struct GTY(()) machine_function
> #define PREDICT_DISTANCE (TARGET_Z10 ? 384 : 2048)
>
>
> +/* Indicate which ABI has been used for passing vector args.
> + 0 - no vector type arguments have been passed where the ABI is relevant
> + 1 - the old ABI has been used
> + 2 - a vector type argument has been passed either in a vector register
> + or on the stack by value */
> +static int s390_vector_abi = 0;
> +
> +/* Set the vector ABI marker if TYPE is subject to the vector ABI
> + switch. The vector ABI affects only vector data types. There are
> + two aspects of the vector ABI relevant here:
> +
> + 1. vectors >= 16 bytes have an alignment of 8 bytes with the new
> + ABI and natural alignment with the old.
> +
> + 2. vector <= 16 bytes are passed in VRs or by value on the stack
> + with the new ABI but by reference on the stack with the old.
> +
> + If ARG_P is true TYPE is used for a function argument or return
> + value. The ABI marker then is set for all vector data types. If
> + ARG_P is false only type 1 vectors are being checked. */
> +
> +static void
> +s390_check_type_for_vector_abi (const_tree type, bool arg_p, bool
> in_struct_p)
> +{
> + static hash_set<const_tree> visited_types_hash;
> +
> + if (s390_vector_abi)
> + return;
> +
> + if (type == NULL_TREE || TREE_CODE (type) == ERROR_MARK)
> + return;
> +
> + if (visited_types_hash.contains (type))
> + return;
> +
> + visited_types_hash.add (type);
> +
> + if (VECTOR_TYPE_P (type))
> + {
> + int type_size = int_size_in_bytes (type);
> +
> + /* Outside arguments only the alignment is changing and this
> + only happens for vector types >= 16 bytes. */
> + if (!arg_p && type_size < 16)
> + return;
> +
> + /* In arguments vector types > 16 are passed as before (GCC
> + never enforced the bigger alignment for arguments which was
> + required by the old vector ABI). However, it might still be
> + ABI relevant due to the changed alignment if it is a struct
> + member. */
> + if (arg_p && type_size > 16 && !in_struct_p)
> + return;
> +
> + s390_vector_abi = TARGET_VX_ABI ? 2 : 1;
> + }
> + else if (POINTER_TYPE_P (type) || TREE_CODE (type) == ARRAY_TYPE)
> + {
> + /* ARRAY_TYPE: Since with neither of the ABIs we have more than
> + natural alignment there will never be ABI dependent padding
> + in an array type. That's why we do not set in_struct_p to
> + true here. */
> + s390_check_type_for_vector_abi (TREE_TYPE (type), arg_p, in_struct_p);
> + }
> + else if (TREE_CODE (type) == FUNCTION_TYPE || TREE_CODE (type) ==
> METHOD_TYPE)
> + {
> + tree arg_chain;
> +
> + /* Check the return type. */
> + s390_check_type_for_vector_abi (TREE_TYPE (type), true, false);
> +
> + for (arg_chain = TYPE_ARG_TYPES (type);
> + arg_chain;
> + arg_chain = TREE_CHAIN (arg_chain))
> + s390_check_type_for_vector_abi (TREE_VALUE (arg_chain), true, false);
> + }
> + else if (RECORD_OR_UNION_TYPE_P (type))
> + {
> + tree field;
> +
> + for (field = TYPE_FIELDS (type); field; field = DECL_CHAIN (field))
> + {
> + if (TREE_CODE (field) != FIELD_DECL)
> + continue;
> +
> + s390_check_type_for_vector_abi (TREE_TYPE (field), arg_p, true);
> + }
> + }
> +}
> +
> +
> /* System z builtins. */
>
> #include "s390-builtins.h"
> @@ -10898,6 +10989,8 @@ s390_function_arg (cumulative_args_t cum_v,
> machine_mode mode,
> {
> CUMULATIVE_ARGS *cum = get_cumulative_args (cum_v);
>
> + if (!named)
> + s390_check_type_for_vector_abi (type, true, false);
>
> if (s390_function_arg_vector (mode, type))
> {
> @@ -11289,6 +11382,8 @@ s390_gimplify_va_arg (tree valist, tree type,
> gimple_seq *pre_p,
>
> size = int_size_in_bytes (type);
>
> + s390_check_type_for_vector_abi (type, true, false);
> +
> if (pass_by_reference (NULL, TYPE_MODE (type), type, false))
> {
> if (TARGET_DEBUG_ARG)
> @@ -13651,6 +13746,29 @@ s390_vector_alignment (const_tree type)
> return MIN (64, tree_to_shwi (TYPE_SIZE (type)));
> }
>
> +/* Implement TARGET_ASM_FILE_END. */
> +static void
> +s390_asm_file_end (void)
> +{
> +#ifdef HAVE_AS_GNU_ATTRIBUTE
> + varpool_node *vnode;
> + cgraph_node *cnode;
> +
> + FOR_EACH_VARIABLE (vnode)
> + if (TREE_PUBLIC (vnode->decl))
> + s390_check_type_for_vector_abi (TREE_TYPE (vnode->decl), false, false);
> +
> + FOR_EACH_FUNCTION (cnode)
> + if (TREE_PUBLIC (cnode->decl))
> + s390_check_type_for_vector_abi (TREE_TYPE (cnode->decl), false, false);
> +
> +
> + if (s390_vector_abi != 0)
> + fprintf (asm_out_file, "\t.gnu_attribute 8, %d\n",
> + s390_vector_abi);
> +#endif
> + file_end_indicate_exec_stack ();
> +}
>
> /* Return true if TYPE is a vector bool type. */
> static inline bool
> @@ -13927,6 +14045,9 @@ s390_invalid_binary_op (int op ATTRIBUTE_UNUSED,
> const_tree type1, const_tree ty
> #undef TARGET_INVALID_BINARY_OP
> #define TARGET_INVALID_BINARY_OP s390_invalid_binary_op
>
> +#undef TARGET_ASM_FILE_END
> +#define TARGET_ASM_FILE_END s390_asm_file_end
> +
> struct gcc_target targetm = TARGET_INITIALIZER;
>
> #include "gt-s390.h"
> diff --git a/gcc/configure b/gcc/configure
> index b26a86f..3f3f578 100755
> --- a/gcc/configure
> +++ b/gcc/configure
> @@ -26708,6 +26708,42 @@ fi
> as_fn_error "Requesting --with-nan= requires assembler support for
> -mnan=" "$LINENO" 5
> fi
> ;;
> + s390*-*-*)
> + { $as_echo "$as_me:${as_lineno-$LINENO}: checking assembler for
> .gnu_attribute support" >&5
> +$as_echo_n "checking assembler for .gnu_attribute support... " >&6; }
> +if test "${gcc_cv_as_s390_gnu_attribute+set}" = set; then :
> + $as_echo_n "(cached) " >&6
> +else
> + gcc_cv_as_s390_gnu_attribute=no
> + if test $in_tree_gas = yes; then
> + if test $gcc_cv_gas_vers -ge `expr \( \( 2 \* 1000 \) + 18 \) \* 1000 +
> 0`
> + then gcc_cv_as_s390_gnu_attribute=yes
> +fi
> + elif test x$gcc_cv_as != x; then
> + $as_echo '.gnu_attribute 8,1' > conftest.s
> + if { ac_try='$gcc_cv_as $gcc_cv_as_flags -o conftest.o conftest.s >&5'
> + { { eval echo "\"\$as_me\":${as_lineno-$LINENO}: \"$ac_try\""; } >&5
> + (eval $ac_try) 2>&5
> + ac_status=$?
> + $as_echo "$as_me:${as_lineno-$LINENO}: \$? = $ac_status" >&5
> + test $ac_status = 0; }; }
> + then
> + gcc_cv_as_s390_gnu_attribute=yes
> + else
> + echo "configure: failed program was" >&5
> + cat conftest.s >&5
> + fi
> + rm -f conftest.o conftest.s
> + fi
> +fi
> +{ $as_echo "$as_me:${as_lineno-$LINENO}: result:
> $gcc_cv_as_s390_gnu_attribute" >&5
> +$as_echo "$gcc_cv_as_s390_gnu_attribute" >&6; }
> +if test $gcc_cv_as_s390_gnu_attribute = yes; then
> +
> +$as_echo "#define HAVE_AS_GNU_ATTRIBUTE 1" >>confdefs.h
> +
> +fi
> + ;;
> esac
>
> # Mips and HP-UX need the GNU assembler.
> diff --git a/gcc/configure.ac b/gcc/configure.ac
> index c09f3ae5..85f72d5 100644
> --- a/gcc/configure.ac
> +++ b/gcc/configure.ac
> @@ -4442,6 +4442,13 @@ pointers into PC-relative form.])
> [Requesting --with-nan= requires assembler support for -mnan=])
> fi
> ;;
> + s390*-*-*)
> + gcc_GAS_CHECK_FEATURE([.gnu_attribute support],
> + gcc_cv_as_s390_gnu_attribute, [2,18,0],,
> + [.gnu_attribute 8,1],,
> + [AC_DEFINE(HAVE_AS_GNU_ATTRIBUTE, 1,
> + [Define if your assembler supports .gnu_attribute.])])
> + ;;
> esac
>
> # Mips and HP-UX need the GNU assembler.
> diff --git a/gcc/testsuite/gcc.target/s390/vector/vec-abi-1.c
> b/gcc/testsuite/gcc.target/s390/vector/vec-abi-1.c
> index 5484664..db18e5e 100644
> --- a/gcc/testsuite/gcc.target/s390/vector/vec-abi-1.c
> +++ b/gcc/testsuite/gcc.target/s390/vector/vec-abi-1.c
> @@ -6,6 +6,7 @@
> /* Make sure the last argument is fetched from the argument overflow area.
> */
> /* { dg-final { scan-assembler "vl\t%v\[0-9\]*,160\\(%r15\\)" { target lp64
> } } } */
> /* { dg-final { scan-assembler "vl\t%v\[0-9\]*,96\\(%r15\\)" { target ilp32
> } } } */
> +/* { dg-final { scan-assembler "gnu_attribute 8, 2" } } */
>
> typedef double v2df __attribute__((vector_size(16)));
>
> diff --git a/gcc/testsuite/gcc.target/s390/vector/vec-abi-attr-1.c
> b/gcc/testsuite/gcc.target/s390/vector/vec-abi-attr-1.c
> new file mode 100644
> index 0000000..a06b338
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/s390/vector/vec-abi-attr-1.c
> @@ -0,0 +1,18 @@
> +/* Check calling convention in the vector ABI. */
> +
> +/* { dg-do compile { target { s390*-*-* } } } */
> +/* { dg-options "-O3 -mzarch -march=z13 -mno-vx" } */
> +
> +/* The function passes arguments whose calling conventions change with
> + -mvx/-mno-vx. In that case GCC has to emit the ABI attribute to
> + allow GDB and Binutils to detect this. */
> +/* { dg-final { scan-assembler "gnu_attribute 8, 1" } } */
> +
> +typedef double v2df __attribute__((vector_size(16)));
> +
> +v2df
> +add (v2df a, v2df b, v2df c, v2df d,
> + v2df e, v2df f, v2df g, v2df h, v2df i)
> +{
> + return a + b + c + d + e + f + g + h + i;
> +}
> diff --git a/gcc/testsuite/gcc.target/s390/vector/vec-abi-attr-2.c
> b/gcc/testsuite/gcc.target/s390/vector/vec-abi-attr-2.c
> new file mode 100644
> index 0000000..97b9748
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/s390/vector/vec-abi-attr-2.c
> @@ -0,0 +1,53 @@
> +/* Check calling convention in the vector ABI. */
> +
> +/* { dg-do compile { target { s390*-*-* } } } */
> +/* { dg-options "-O3 -mzarch -march=z13" } */
> +
> +/* No abi attribute should be emitted when nothing relevant happened. */
> +/* { dg-final { scan-assembler-not "gnu_attribute" } } */
> +
> +#include <stdarg.h>
> +
> +/* Local use is ok. */
> +
> +typedef int v4si __attribute__((vector_size(16)));
> +
> +static
> +v4si __attribute__((__noinline__))
> +foo (v4si a)
> +{
> + return a + (v4si){ 1, 2, 3, 4 };
> +}
> +
> +int
> +bar (int a)
> +{
> + return foo ((v4si){ 1, 1, 1, 1 })[1];
> +}
> +
> +/* Big vector type only used as function argument and return value
> + without being a struct/union member. The alignment change is not
> + relevant here. */
> +
> +typedef double v4df __attribute__((vector_size(32)));
> +
> +v4df
> +add (v4df a, v4df b, v4df c, v4df d,
> + v4df e, v4df f, v4df g, v4df h, v4df i)
> +{
> + return a + b + c + d + e + f + g + h + i;
> +}
> +
> +double
> +bar2 (int n, ...)
> +{
> + double ret;
> + v4df a;
> + va_list va;
> +
> + va_start (va, n);
> + ret = va_arg (va, v4df)[2];
> + va_end (va);
> +
> + return ret;
> +}
> diff --git a/gcc/testsuite/gcc.target/s390/vector/vec-abi-attr-3.c
> b/gcc/testsuite/gcc.target/s390/vector/vec-abi-attr-3.c
> new file mode 100644
> index 0000000..f3dc368
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/s390/vector/vec-abi-attr-3.c
> @@ -0,0 +1,18 @@
> +/* Check calling convention in the vector ABI. */
> +
> +/* { dg-do compile { target { s390*-*-* } } } */
> +/* { dg-options "-O3 -mzarch -march=z13" } */
> +
> +/* { dg-final { scan-assembler "gnu_attribute 8, 2" } } */
> +
> +typedef double v4df __attribute__((vector_size(32)));
> +typedef struct { v4df a; } s;
> +
> +s
> +add (v4df a, v4df b, v4df c, v4df d,
> + v4df e, v4df f, v4df g, v4df h, v4df i)
> +{
> + s t;
> + t.a = a + b + c + d + e + f + g + h + i;
> + return t;
> +}
> diff --git a/gcc/testsuite/gcc.target/s390/vector/vec-abi-attr-4.c
> b/gcc/testsuite/gcc.target/s390/vector/vec-abi-attr-4.c
> new file mode 100644
> index 0000000..ad9b29a
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/s390/vector/vec-abi-attr-4.c
> @@ -0,0 +1,17 @@
> +/* Check calling convention in the vector ABI. */
> +
> +/* { dg-do compile { target { s390*-*-* } } } */
> +/* { dg-options "-O3 -mzarch -march=z13" } */
> +
> +/* { dg-final { scan-assembler "gnu_attribute 8, 2" } } */
> +
> +typedef int __attribute__((vector_size(16))) v4si;
> +
> +extern void bar (v4si);
> +
> +void
> +foo (int a)
> +{
> + v4si b = (v4si){ a, a, a, a };
> + bar (b);
> +}
> diff --git a/gcc/testsuite/gcc.target/s390/vector/vec-abi-attr-5.c
> b/gcc/testsuite/gcc.target/s390/vector/vec-abi-attr-5.c
> new file mode 100644
> index 0000000..fb5de4e
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/s390/vector/vec-abi-attr-5.c
> @@ -0,0 +1,19 @@
> +/* Check calling convention in the vector ABI. */
> +
> +/* { dg-do compile { target { s390*-*-* } } } */
> +/* { dg-options "-O3 -mzarch -march=z13" } */
> +
> +/* { dg-final { scan-assembler "gnu_attribute 8, 2" } } */
> +
> +#include <stdarg.h>
> +
> +typedef int __attribute__((vector_size(16))) v4si;
> +
> +extern void bar (int, ...);
> +
> +void
> +foo (int a)
> +{
> + v4si b = (v4si){ a, a, a, a };
> + bar (1, b);
> +}
> diff --git a/gcc/testsuite/gcc.target/s390/vector/vec-abi-attr-6.c
> b/gcc/testsuite/gcc.target/s390/vector/vec-abi-attr-6.c
> new file mode 100644
> index 0000000..9134fa7
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/s390/vector/vec-abi-attr-6.c
> @@ -0,0 +1,24 @@
> +/* Check calling convention in the vector ABI. */
> +
> +/* { dg-do compile { target { s390*-*-* } } } */
> +/* { dg-options "-O3 -mzarch -march=z13" } */
> +
> +/* { dg-final { scan-assembler "gnu_attribute 8, 2" } } */
> +
> +#include <stdarg.h>
> +
> +typedef int __attribute__((vector_size(16))) v4si;
> +
> +int
> +bar (int n, ...)
> +{
> + int ret;
> + v4si a;
> + va_list va;
> +
> + va_start (va, n);
> + ret = va_arg (va, v4si)[2];
> + va_end (va);
> +
> + return ret;
> +}
> --
> 1.7.9.5
>
