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
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);
}
