diff mbox

[PR70955] Tag {ms,sysv}_va_list_type_node with {ms,sysv}_abi attribute

Message ID 0b95bce7-65c8-b122-9125-9e7a19041df3@mentor.com
State New
Headers show

Commit Message

Tom de Vries Aug. 24, 2016, 6:28 p.m. UTC
Hi,

in PR70955, ix86_canonical_va_list_type fails to recognize a 
__builtin_ms_va_list that was declared in a TU, because its type doesn't 
have the same main variant as the ms_va_list_type_node initialized in lto1.

This patch fixes the PR by tagging ms_va_list_type_node and 
sysv_va_list_type_node with ms_abi/sysv_abi attributes.

sysv_va_list_type_node is of type array of length one with elemtype 
record, and I ran into trouble with both adding the attribute to the 
array type and the record type, so I ended up adding it to the first 
field type.

Bootstrapped and reg-tested on x86_64.

OK for trunk, 6 branch?

Thanks,
- Tom

Comments

Richard Biener Aug. 25, 2016, 11:48 a.m. UTC | #1
On Wed, 24 Aug 2016, Tom de Vries wrote:

> Hi,
> 
> in PR70955, ix86_canonical_va_list_type fails to recognize a
> __builtin_ms_va_list that was declared in a TU, because its type doesn't have
> the same main variant as the ms_va_list_type_node initialized in lto1.
> 
> This patch fixes the PR by tagging ms_va_list_type_node and
> sysv_va_list_type_node with ms_abi/sysv_abi attributes.
> 
> sysv_va_list_type_node is of type array of length one with elemtype record,
> and I ran into trouble with both adding the attribute to the array type and
> the record type, so I ended up adding it to the first field type.
> 
> Bootstrapped and reg-tested on x86_64.
> 
> OK for trunk, 6 branch?

I miss some comments in the code laying out the scheme to identify
the types.

How did you build the sysv_abi tagged struct / array that ended up
not working?  I suspect that in parameter passing the array somehow
decays to a pointer-to-element type so it is important that the
TYPE_MAIN_VARIANT of the record type already contains the attribute.

We've had a first/second testcase in the PR but you only added one - does
it cover both cases?

Thanks,
Richard.
diff mbox

Patch

Tag {ms,sysv}_va_list_type_node with {ms,sysv}_abi attribute

2016-08-24  Tom de Vries  <tom@codesourcery.com>

	PR lto/70955
	* config/i386/i386.c (ix86_build_builtin_va_list_64): Tag type with
	sysv_abi attribute.
	(ix86_build_builtin_va_list): Tag type with ms_abi attribute.
	(ix86_canonical_va_list_type): Handle sysv_abi and ms_abi attributes.

	* gcc.dg/pr70955.c: New test.

---
 gcc/config/i386/i386.c         | 89 +++++++++++++-----------------------------
 gcc/testsuite/gcc.dg/pr70955.c | 33 ++++++++++++++++
 2 files changed, 61 insertions(+), 61 deletions(-)

diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index 2639c8c..b5a5bd1 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -10518,9 +10518,13 @@  ix86_build_builtin_va_list_64 (void)
   type_decl = build_decl (BUILTINS_LOCATION,
 			  TYPE_DECL, get_identifier ("__va_list_tag"), record);
 
+  tree attr = tree_cons (get_identifier ("sysv_abi"), NULL_TREE, NULL_TREE);
+  tree sysv_unsigned_type_node
+    = build_type_attribute_variant (unsigned_type_node, attr);
+
   f_gpr = build_decl (BUILTINS_LOCATION,
 		      FIELD_DECL, get_identifier ("gp_offset"),
-		      unsigned_type_node);
+		      sysv_unsigned_type_node);
   f_fpr = build_decl (BUILTINS_LOCATION,
 		      FIELD_DECL, get_identifier ("fp_offset"),
 		      unsigned_type_node);
@@ -10561,16 +10565,16 @@  ix86_build_builtin_va_list (void)
   if (TARGET_64BIT)
     {
       /* Initialize ABI specific va_list builtin types.  */
-      tree sysv_va_list, ms_va_list;
-
-      sysv_va_list = ix86_build_builtin_va_list_64 ();
-      sysv_va_list_type_node = build_variant_type_copy (sysv_va_list);
+      sysv_va_list_type_node = ix86_build_builtin_va_list_64 ();
 	
       /* For MS_ABI we use plain pointer to argument area.  */
-      ms_va_list = build_pointer_type (char_type_node);
-      ms_va_list_type_node = build_variant_type_copy (ms_va_list);
+      tree char_ptr_type = build_pointer_type (char_type_node);
+      tree attr = tree_cons (get_identifier ("ms_abi"), NULL_TREE, NULL_TREE);
+      ms_va_list_type_node = build_type_attribute_variant (char_ptr_type, attr);
 
-      return (ix86_abi == MS_ABI) ? ms_va_list : sysv_va_list;
+      return ((ix86_abi == MS_ABI)
+	      ? ms_va_list_type_node
+	      : sysv_va_list_type_node);
     }
   else
     {
@@ -48563,8 +48567,6 @@  ix86_fn_abi_va_list (tree fndecl)
 static tree
 ix86_canonical_va_list_type (tree type)
 {
-  tree wtype, htype;
-
   /* Resolve references and pointers to va_list type.  */
   if (TREE_CODE (type) == MEM_REF)
     type = TREE_TYPE (type);
@@ -48573,64 +48575,29 @@  ix86_canonical_va_list_type (tree type)
   else if (POINTER_TYPE_P (type) && TREE_CODE (TREE_TYPE (type)) == ARRAY_TYPE)
     type = TREE_TYPE (type);
 
-  if (TARGET_64BIT && va_list_type_node != NULL_TREE)
+  if (TARGET_64BIT)
     {
-      wtype = va_list_type_node;
-	  gcc_assert (wtype != NULL_TREE);
-      htype = type;
-      if (TREE_CODE (wtype) == ARRAY_TYPE)
-	{
-	  /* If va_list is an array type, the argument may have decayed
-	     to a pointer type, e.g. by being passed to another function.
-	     In that case, unwrap both types so that we can compare the
-	     underlying records.  */
-	  if (TREE_CODE (htype) == ARRAY_TYPE
-	      || POINTER_TYPE_P (htype))
-	    {
-	      wtype = TREE_TYPE (wtype);
-	      htype = TREE_TYPE (htype);
-	    }
-	}
-      if (TYPE_MAIN_VARIANT (wtype) == TYPE_MAIN_VARIANT (htype))
-	return va_list_type_node;
-      wtype = sysv_va_list_type_node;
-	  gcc_assert (wtype != NULL_TREE);
-      htype = type;
-      if (TREE_CODE (wtype) == ARRAY_TYPE)
-	{
-	  /* If va_list is an array type, the argument may have decayed
-	     to a pointer type, e.g. by being passed to another function.
-	     In that case, unwrap both types so that we can compare the
-	     underlying records.  */
-	  if (TREE_CODE (htype) == ARRAY_TYPE
-	      || POINTER_TYPE_P (htype))
-	    {
-	      wtype = TREE_TYPE (wtype);
-	      htype = TREE_TYPE (htype);
-	    }
-	}
-      if (TYPE_MAIN_VARIANT (wtype) == TYPE_MAIN_VARIANT (htype))
-	return sysv_va_list_type_node;
-      wtype = ms_va_list_type_node;
-	  gcc_assert (wtype != NULL_TREE);
-      htype = type;
-      if (TREE_CODE (wtype) == ARRAY_TYPE)
+      if (lookup_attribute ("ms_abi", TYPE_ATTRIBUTES (type)))
+	return ms_va_list_type_node;
+
+      if ((TREE_CODE (type) == ARRAY_TYPE
+	   && integer_zerop (array_type_nelts (type)))
+	  || POINTER_TYPE_P (type))
 	{
-	  /* If va_list is an array type, the argument may have decayed
-	     to a pointer type, e.g. by being passed to another function.
-	     In that case, unwrap both types so that we can compare the
-	     underlying records.  */
-	  if (TREE_CODE (htype) == ARRAY_TYPE
-	      || POINTER_TYPE_P (htype))
+	  tree elem_type = TREE_TYPE (type);
+	  tree first_field;
+	  if (TREE_CODE (elem_type) == RECORD_TYPE
+	      && (first_field = TYPE_FIELDS (elem_type)))
 	    {
-	      wtype = TREE_TYPE (wtype);
-	      htype = TREE_TYPE (htype);
+	      tree attrs = TYPE_ATTRIBUTES (TREE_TYPE (first_field));
+	      if (lookup_attribute ("sysv_abi", attrs))
+		return sysv_va_list_type_node;
 	    }
 	}
-      if (TYPE_MAIN_VARIANT (wtype) == TYPE_MAIN_VARIANT (htype))
-	return ms_va_list_type_node;
+
       return NULL_TREE;
     }
+
   return std_canonical_va_list_type (type);
 }
 
diff --git a/gcc/testsuite/gcc.dg/pr70955.c b/gcc/testsuite/gcc.dg/pr70955.c
new file mode 100644
index 0000000..11685c8
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr70955.c
@@ -0,0 +1,33 @@ 
+/* { dg-do run } */
+/* { dg-require-effective-target lto } */
+/* { dg-options "-flto" } */
+
+#include <stdio.h>
+
+int __attribute__((ms_abi))
+foo (int n, ...)
+{
+  __builtin_ms_va_list ap;
+  int sum = 0;
+
+  __builtin_ms_va_start (ap, n);
+
+  while (n--) {
+    sum += __builtin_va_arg (ap, int);
+    printf("sum = %d\n", sum);
+  }
+  __builtin_ms_va_end (ap);
+
+  return sum;
+}
+
+int
+main ()
+{
+  int res = foo (10, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10);
+
+  if (res != 55)
+    __builtin_abort ();
+
+  return 0;
+}