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.
