This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[RFC 12/13] S/390 Vector ABI GNU Attribute.


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.

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                             |  120 ++++++++++++++++++++
 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, 313 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 e682516..e1ae1ed 100644
--- a/gcc/config/s390/s390.c
+++ b/gcc/config/s390/s390.c
@@ -466,6 +466,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"
@@ -10900,6 +10991,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))
     {
@@ -11292,6 +11385,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)
@@ -13646,6 +13741,28 @@ 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
+}
 
 /* Initialize GCC target structure.  */
 
@@ -13861,6 +13978,9 @@ s390_vector_alignment (const_tree type)
 #undef TARGET_VECTOR_ALIGNMENT
 #define TARGET_VECTOR_ALIGNMENT s390_vector_alignment
 
+#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 9523773..a6d8e11 100755
--- a/gcc/configure
+++ b/gcc/configure
@@ -26544,6 +26544,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 68b0ee8..577437e 100644
--- a/gcc/configure.ac
+++ b/gcc/configure.ac
@@ -4426,6 +4426,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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]