diff mbox

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

Message ID eeef05d8-77a3-20f0-999b-269e4e2d1178@mentor.com
State New
Headers show

Commit Message

Tom de Vries Aug. 26, 2016, 3:33 p.m. UTC
On 26/08/16 10:40, Richard Biener wrote:
> I suppose
> you could get away with using sth entirely private to the backend,
> like "sysv abi valist", also not ever user-creatable.  Does this
> side-step the FE issue?

It does.

Added comments and second test-case, as requested previously.

Bootstrapped and reg-tested on x86_64 -m64/-m32.

OK for trunk, 6 branch?

Thanks,
- Tom

Comments

Richard Biener Aug. 27, 2016, 12:51 p.m. UTC | #1
On Fri, Aug 26, 2016 at 5:33 PM, Tom de Vries <Tom_deVries@mentor.com> wrote:
> On 26/08/16 10:40, Richard Biener wrote:
>>
>> I suppose
>> you could get away with using sth entirely private to the backend,
>> like "sysv abi valist", also not ever user-creatable.  Does this
>> side-step the FE issue?
>
>
> It does.
>
> Added comments and second test-case, as requested previously.
>
> Bootstrapped and reg-tested on x86_64 -m64/-m32.
>
> OK for trunk, 6 branch?

Ok.

Thanks,
Richard.

>
> Thanks,
> - Tom
Markus Trippelsdorf Sept. 1, 2016, 12:25 p.m. UTC | #2
On 2016.08.26 at 17:33 +0200, Tom de Vries wrote:
> On 26/08/16 10:40, Richard Biener wrote:
> >I suppose
> >you could get away with using sth entirely private to the backend,
> >like "sysv abi valist", also not ever user-creatable.  Does this
> >side-step the FE issue?
> 
> It does.
> 
> Added comments and second test-case, as requested previously.
> 
> Bootstrapped and reg-tested on x86_64 -m64/-m32.
> 
> OK for trunk, 6 branch?

The patch causes: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=77427
diff mbox

Patch

Tag {ms,sysv}_va_list_type_node with '{ms,sysv}_abi va_list' 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 va_list' attribute.
	(ix86_build_builtin_va_list): Tag type with 'ms_abi va_list' attribute.
	(ix86_canonical_va_list_type): Handle 'sysv_abi/ms_abi va_list'
	attributes.

	* gcc.dg/pr70955.c: New test.
	* gcc.dg/lto/pr70955_0.c: Same.
	* gcc.dg/lto/pr70955_1.c: Same.

---
 gcc/config/i386/i386.c               | 109 +++++++++++++++--------------------
 gcc/testsuite/gcc.dg/lto/pr70955_0.c |  13 +++++
 gcc/testsuite/gcc.dg/lto/pr70955_1.c |  16 +++++
 gcc/testsuite/gcc.dg/pr70955.c       |  36 ++++++++++++
 4 files changed, 111 insertions(+), 63 deletions(-)

diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index 2639c8c..792019b 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -10548,6 +10548,9 @@  ix86_build_builtin_va_list_64 (void)
 
   layout_type (record);
 
+  TYPE_ATTRIBUTES (record) = tree_cons (get_identifier ("sysv_abi va_list"),
+					NULL_TREE, TYPE_ATTRIBUTES (record));
+
   /* The correct type is an array type of one element.  */
   return build_array_type (record, build_index_type (size_zero_node));
 }
@@ -10560,17 +10563,36 @@  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);
+      /* Initialize ABI specific va_list builtin types.
+
+	 In lto1, we can encounter two va_list types:
+	 - one as a result of the type-merge across TUs, and
+	 - the one constructed here.
+	 These two types will not have the same TYPE_MAIN_VARIANT, and therefore
+	 a type identity check in canonical_va_list_type based on
+	 TYPE_MAIN_VARIANT (which we used to have) will not work.
+	 Instead, we tag each va_list_type_node with its unique attribute, and
+	 look for the attribute in the type identity check in
+	 canonical_va_list_type.
+
+	 Tagging sysv_va_list_type_node directly with the attribute is
+	 problematic since it's a array of one record, which will degrade into a
+	 pointer to record when used as parameter (see build_va_arg comments for
+	 an example), dropping the attribute in the process.  So we tag the
+	 record instead.  */
+
+      /* For SYSV_ABI we use an array of one record.  */
+      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 va_list"), NULL_TREE,
+			     TYPE_ATTRIBUTES (char_ptr_type));
+      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
     {
@@ -44669,6 +44691,8 @@  static const struct attribute_spec ix86_attribute_table[] =
   /* ms_abi and sysv_abi calling convention function attributes.  */
   { "ms_abi", 0, 0, false, true, true, ix86_handle_abi_attribute, true },
   { "sysv_abi", 0, 0, false, true, true, ix86_handle_abi_attribute, true },
+  { "ms_abi va_list", 0, 0, false, false, false, NULL, false },
+  { "sysv_abi va_list", 0, 0, false, false, false, NULL, false },
   { "ms_hook_prologue", 0, 0, true, false, false, ix86_handle_fndecl_attribute,
     false },
   { "callee_pop_aggregate_return", 1, 1, false, true, true,
@@ -48563,8 +48587,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 +48595,25 @@  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 va_list", 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))
-	    {
-	      wtype = TREE_TYPE (wtype);
-	      htype = TREE_TYPE (htype);
-	    }
+	  tree elem_type = TREE_TYPE (type);
+	  if (TREE_CODE (elem_type) == RECORD_TYPE
+	      && lookup_attribute ("sysv_abi va_list",
+				   TYPE_ATTRIBUTES (elem_type)))
+	    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/lto/pr70955_0.c b/gcc/testsuite/gcc.dg/lto/pr70955_0.c
new file mode 100644
index 0000000..c3b75fc
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/lto/pr70955_0.c
@@ -0,0 +1,13 @@ 
+/* __builtin_ms_va_list is only supported for x86_64 -m64.  */
+/* { dg-skip-if "" { ! {x86_64-*-* && { ! ilp32 } } } } */
+
+#include <stdio.h>
+
+int __attribute__((ms_abi)) va_demo (int count, ...);
+
+int
+main (void)
+{
+  printf ("sum == %d\n", va_demo (5, 1, 2, 3, 4, 5));
+  return 0;
+}
diff --git a/gcc/testsuite/gcc.dg/lto/pr70955_1.c b/gcc/testsuite/gcc.dg/lto/pr70955_1.c
new file mode 100644
index 0000000..204c28b
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/lto/pr70955_1.c
@@ -0,0 +1,16 @@ 
+int __attribute__((ms_abi))
+va_demo (int count, ...)
+{
+  int sum = 0;
+  __builtin_ms_va_list ap;
+
+  __builtin_ms_va_start (ap, count);
+  while (count)
+    {
+      sum += __builtin_va_arg (ap, int);
+      --count;
+    }
+
+  __builtin_ms_va_end (ap);
+  return sum;
+}
diff --git a/gcc/testsuite/gcc.dg/pr70955.c b/gcc/testsuite/gcc.dg/pr70955.c
new file mode 100644
index 0000000..1275a5f
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr70955.c
@@ -0,0 +1,36 @@ 
+/* __builtin_ms_va_list is only supported for x86_64 -m64.  */
+/* { dg-do run { target { x86_64-*-* && { ! ilp32 } } } } */
+/* { 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 (void)
+{
+  int res = foo (10, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10);
+
+  if (res != 55)
+    __builtin_abort ();
+
+  return 0;
+}