* [PATCH, PR70955] Tag {ms,sysv}_va_list_type_node with {ms,sysv}_abi attribute
@ 2016-08-24 18:29 Tom de Vries
2016-08-25 11:48 ` Richard Biener
0 siblings, 1 reply; 7+ messages in thread
From: Tom de Vries @ 2016-08-24 18:29 UTC (permalink / raw)
To: Richard Biener; +Cc: GCC Patches
[-- Attachment #1: Type: text/plain, Size: 637 bytes --]
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
[-- Attachment #2: 0001-Tag-ms-sysv-_va_list_type_node-with-ms-sysv-_abi-attribute.patch --]
[-- Type: text/x-patch, Size: 6106 bytes --]
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;
+}
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH, PR70955] Tag {ms,sysv}_va_list_type_node with {ms,sysv}_abi attribute
2016-08-24 18:29 [PATCH, PR70955] Tag {ms,sysv}_va_list_type_node with {ms,sysv}_abi attribute Tom de Vries
@ 2016-08-25 11:48 ` Richard Biener
2016-08-25 13:36 ` Tom de Vries
0 siblings, 1 reply; 7+ messages in thread
From: Richard Biener @ 2016-08-25 11:48 UTC (permalink / raw)
To: Tom de Vries; +Cc: GCC Patches
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.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH, PR70955] Tag {ms,sysv}_va_list_type_node with {ms,sysv}_abi attribute
2016-08-25 11:48 ` Richard Biener
@ 2016-08-25 13:36 ` Tom de Vries
2016-08-26 8:40 ` Richard Biener
0 siblings, 1 reply; 7+ messages in thread
From: Tom de Vries @ 2016-08-25 13:36 UTC (permalink / raw)
To: Richard Biener; +Cc: GCC Patches
[-- Attachment #1: Type: text/plain, Size: 2803 bytes --]
On 25/08/16 13:48, Richard Biener wrote:
> 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?
> How did you build the sysv_abi tagged struct / array that ended up
> not working?
My first try to tag the struct was this:
...
diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index 2639c8c..f07d9f2 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);
+ tree attr = tree_cons (get_identifier ("sysv_abi"), NULL_TREE,
NULL_TREE);
+ record = build_type_attribute_variant (record, attr);
+
/* The correct type is an array type of one element. */
return build_array_type (record, build_index_type (size_zero_node));
}
...
But we immediately run into:
...
<built-in>: warning: ignoring attributes applied to ‘__va_list_tag’
after definition [-Wattributes]
<built-in>: warning: ignoring attributes applied to ‘struct ’ after
definition [-Wattributes]
<built-in>: warning: ignoring attributes applied to ‘struct ’ after
definition [-Wattributes]
...
I tried to work around that by directly assigning to TYPE_ATTRIBUTES, as
implemented in attached patch. But then I run into a libstdc++ build
ICE, in mangle.c:write_type:
...
tree t = TYPE_MAIN_VARIANT (type);
if (TYPE_ATTRIBUTES (t) && !OVERLOAD_TYPE_P (t))
{
tree attrs = NULL_TREE;
if (tx_safe_fn_type_p (type))
attrs = tree_cons (get_identifier ("transaction_safe"),
NULL_TREE, attrs);
t = cp_build_type_attribute_variant (t, attrs);
}
gcc_assert (t != type);
...
> I suspect that in parameter passing the array somehow
> decays to a pointer-to-element type
Yep, as mentioned in more detail at
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=70955#c23 . That's why I
didn't try very hard to get tagging the array with the attribute to work.
> so it is important that the
> TYPE_MAIN_VARIANT of the record type already contains the attribute.
AFAIU, the record type is its own TYPE_MAIN_VARIANT, so it contains the
attributes.
Thanks,
- Tom
[-- Attachment #2: tmp.patch --]
[-- Type: text/x-patch, Size: 4393 bytes --]
diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index 2639c8c..54a0ef9 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -10548,6 +10548,8 @@ ix86_build_builtin_va_list_64 (void)
layout_type (record);
+ TYPE_ATTRIBUTES (record) = tree_cons (get_identifier ("sysv_abi"), NULL_TREE, NULL_TREE);
+
/* The correct type is an array type of one element. */
return build_array_type (record, build_index_type (size_zero_node));
}
@@ -10561,16 +10563,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 +48565,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 +48573,24 @@ 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))
- {
- 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", 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);
}
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH, PR70955] Tag {ms,sysv}_va_list_type_node with {ms,sysv}_abi attribute
2016-08-25 13:36 ` Tom de Vries
@ 2016-08-26 8:40 ` Richard Biener
2016-08-26 15:33 ` Tom de Vries
0 siblings, 1 reply; 7+ messages in thread
From: Richard Biener @ 2016-08-26 8:40 UTC (permalink / raw)
To: Tom de Vries; +Cc: GCC Patches
[-- Attachment #1: Type: TEXT/PLAIN, Size: 3149 bytes --]
On Thu, 25 Aug 2016, Tom de Vries wrote:
> On 25/08/16 13:48, Richard Biener wrote:
> > 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?
>
> > How did you build the sysv_abi tagged struct / array that ended up
> > not working?
>
> My first try to tag the struct was this:
> ...
> diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
> index 2639c8c..f07d9f2 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);
>
> + tree attr = tree_cons (get_identifier ("sysv_abi"), NULL_TREE, NULL_TREE);
> + record = build_type_attribute_variant (record, attr);
> +
> /* The correct type is an array type of one element. */
> return build_array_type (record, build_index_type (size_zero_node));
> }
> ...
>
> But we immediately run into:
> ...
> <built-in>: warning: ignoring attributes applied to ‘__va_list_tag’ after
> definition [-Wattributes]
> <built-in>: warning: ignoring attributes applied to ‘struct ’ after definition
> [-Wattributes]
> <built-in>: warning: ignoring attributes applied to ‘struct ’ after definition
> [-Wattributes]
> ...
Yeah, and this also builds a variant type.
>
> I tried to work around that by directly assigning to TYPE_ATTRIBUTES, as
> implemented in attached patch.
I expected that this would work ...
> But then I run into a libstdc++ build ICE, in
> mangle.c:write_type:
> ...
> tree t = TYPE_MAIN_VARIANT (type);
> if (TYPE_ATTRIBUTES (t) && !OVERLOAD_TYPE_P (t))
> {
> tree attrs = NULL_TREE;
> if (tx_safe_fn_type_p (type))
> attrs = tree_cons (get_identifier ("transaction_safe"),
> NULL_TREE, attrs);
> t = cp_build_type_attribute_variant (t, attrs);
> }
> gcc_assert (t != type);
> ...
Hmm, you shouldn't run into write_CV_qualifiers_for_type as the
type shouldn't be CV qualified (and any CV qualified record type
should be indeed a variant type of the unqualified record).
Ah, so write_CV_qualifiers_for_type looks at ABI changing attributes
and it seems yours counts as such (because it is a documented
attribute even only documented as function attribute). 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?
Richard.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH, PR70955] Tag {ms,sysv}_va_list_type_node with {ms,sysv}_abi attribute
2016-08-26 8:40 ` Richard Biener
@ 2016-08-26 15:33 ` Tom de Vries
2016-08-27 12:51 ` Richard Biener
2016-09-01 12:26 ` Markus Trippelsdorf
0 siblings, 2 replies; 7+ messages in thread
From: Tom de Vries @ 2016-08-26 15:33 UTC (permalink / raw)
To: Richard Biener; +Cc: GCC Patches
[-- Attachment #1: Type: text/plain, Size: 378 bytes --]
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
[-- Attachment #2: 0001-Tag-ms-sysv-_va_list_type_node-with-ms-sysv-_abi-va_list-attribute.patch --]
[-- Type: text/x-patch, Size: 8609 bytes --]
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;
+}
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH, PR70955] Tag {ms,sysv}_va_list_type_node with {ms,sysv}_abi attribute
2016-08-26 15:33 ` Tom de Vries
@ 2016-08-27 12:51 ` Richard Biener
2016-09-01 12:26 ` Markus Trippelsdorf
1 sibling, 0 replies; 7+ messages in thread
From: Richard Biener @ 2016-08-27 12:51 UTC (permalink / raw)
To: Tom de Vries; +Cc: Richard Biener, GCC Patches
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
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH, PR70955] Tag {ms,sysv}_va_list_type_node with {ms,sysv}_abi attribute
2016-08-26 15:33 ` Tom de Vries
2016-08-27 12:51 ` Richard Biener
@ 2016-09-01 12:26 ` Markus Trippelsdorf
1 sibling, 0 replies; 7+ messages in thread
From: Markus Trippelsdorf @ 2016-09-01 12:26 UTC (permalink / raw)
To: Tom de Vries; +Cc: Richard Biener, GCC Patches
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
--
Markus
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2016-09-01 12:26 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-08-24 18:29 [PATCH, PR70955] Tag {ms,sysv}_va_list_type_node with {ms,sysv}_abi attribute Tom de Vries
2016-08-25 11:48 ` Richard Biener
2016-08-25 13:36 ` Tom de Vries
2016-08-26 8:40 ` Richard Biener
2016-08-26 15:33 ` Tom de Vries
2016-08-27 12:51 ` Richard Biener
2016-09-01 12:26 ` Markus Trippelsdorf
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).