This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: Adjust empty class parameter passing ABI (PR c++/60336)


On November 7, 2017 7:21:17 PM GMT+01:00, Jason Merrill <jason@redhat.com> wrote:
>On Tue, Nov 7, 2017 at 3:14 AM, Richard Biener <rguenther@suse.de>
>wrote:
>> On Mon, 6 Nov 2017, Jason Merrill wrote:
>>
>>> On Mon, Nov 6, 2017 at 10:37 AM, Marek Polacek <polacek@redhat.com>
>wrote:
>>> > On Fri, Nov 03, 2017 at 12:19:05PM -0400, Jason Merrill wrote:
>>> >> On Fri, Nov 3, 2017 at 9:55 AM, Marek Polacek
><polacek@redhat.com> wrote:
>>> >> > +  TYPE_EMPTY_P (t) = targetm.calls.empty_record_p (t);
>>> >>
>>> >> I think we want to set this in finalize_type_size; since the
>point of
>>> >> all this is becoming compliant with the psABI (and compatible
>with the
>>> >> C front end), I wouldn't think it should be specific to the C++
>front
>>> >> end.
>>> >
>>> > Sure, that works.  I'd wanted to do the TYPE_EMPTY_P setting in
>layout_type,
>>> > but that wasn't called for the classes in my testcases.  I've
>moved the
>>> > setting of TYPE_EMPTY_P to finalize_type_size now.
>>> >
>>> >> > +  TYPE_WARN_EMPTY_P (t) = warn_abi && abi_version_crosses
>(12);
>>> >>
>>> >> Can this flag go on the TRANSLATION_UNIT_DECL rather than the
>type?
>>> >
>>> > Yeah, that works, too.  To avoid needing a lang hook, I'm setting
>the
>>> > flag in cxx_init_decl_processing, hope that's ok.
>>>
>>> Sure.
>>>
>>> >> > + if (TREE_CODE (field) == FIELD_DECL
>>> >> > +     && (DECL_NAME (field)
>>> >> > +         || RECORD_OR_UNION_TYPE_P (TREE_TYPE (field)))
>>> >> > +     && !default_is_empty_type (TREE_TYPE (field)))
>>> >> > +    return false;
>>> >> > +  return true;
>>> >>
>>> >> Hmm, this assumes that any unnamed field can be ignored; I'm
>concerned
>>> >> that some front end might clear DECL_NAME for a reason that
>doesn't
>>> >> imply that the field is just padding.
>>> >
>>> > In that case I guess we need a new lang hook, right?  Because in
>>> > default_is_empty_type we can't check FE-specific flags such as
>>> > DECL_C_BIT_FIELD.  For C++, should that lang hook be just
>>> > is_really_empty_class?  Or is there anything else I can do?
>>>
>>> Hmm, maybe leave it as it is and just document this assumption about
>>> FIELD_DECL with null DECL_NAME, both here and in tree.def.
>>
>> But are you sure you are not changing the ABI for a language other
>> than C++ then?
>
>No, that is the concern, we'd need to check that.
>
>> I don't think DECL_NAME has any special meaning - why
>> not check DECL_ARTIFICIAL or another flag that has appropriate
>> semantic - _what_ semantic are you looking for after all?
>
>That a struct consisting only of padding is considered empty.
>
>Perhaps we could use decl_flag_3 on FIELD_DECL for DECL_PADDING_P to
>indicate that a field isn't really data.  Or perhaps we should remove
>such things from TYPE_FIELDS after layout is done.

Hmm, I thought we had custom layout routines if you need padding that isn't indicated by the real fields alignment? Why does C++ need explicit FIELD_DECLs here? 

Richard. 

>Jason


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]