r350920 - [Sema] Make canPassInRegisters return true if the CXXRecordDecl passed

Richard Smith via cfe-commits cfe-commits at lists.llvm.org
Thu Jan 31 15:52:49 PST 2019


Given that there's uncertainty as to how to proceed and this patch
affects the ABI, I would prefer that we revert it for trunk and 8.0.

The suggested alternative of disallowing trivial_abi in the case where
all copy/move constructors are deleted seems reasonable to me.

On Thu, 31 Jan 2019 at 14:31, Shoaib Meenai via cfe-commits
<cfe-commits at lists.llvm.org> wrote:
>
> Just wanted to point out that r350920 is on the 8.0 release branch as well. I don't know if there are any additional considerations there.
>
> On 1/31/19, 2:20 PM, "cfe-commits on behalf of John McCall via cfe-commits" <cfe-commits-bounces at lists.llvm.org on behalf of cfe-commits at lists.llvm.org> wrote:
>
>
>
>     On 31 Jan 2019, at 16:57, Akira Hatanaka wrote:
>
>     > Would it be better if we disallowed trivial_abi if the class’ copy
>     > and move destructors were all deleted (and revert r350920)? I think
>     > that would make it easier to reason about when you are allowed to use
>     > trivial_abi and what effect the attribute has (which is to override
>     > the trivialness for the purpose of calls).
>     >
>     > Sorry for my late reply. It took a while to understand that the patch
>     > I committed might not be the right way to fix the problem.
>
>     I'd be fine with that.  If nothing else, we can generalize it later if
>     we decide that's an important use-case.
>
>     John.
>
>     >
>     >> On Jan 16, 2019, at 8:37 PM, John McCall via cfe-commits
>     >> <cfe-commits at lists.llvm.org> wrote:
>     >>
>     >> On 16 Jan 2019, at 20:03, Richard Smith wrote:
>     >>
>     >>> On Wed, 16 Jan 2019 at 16:20, John McCall via cfe-commits <
>     >>> cfe-commits at lists.llvm.org> wrote:
>     >>>
>     >>>> On 16 Jan 2019, at 18:32, Richard Smith wrote:
>     >>>>
>     >>>>> On Wed, 16 Jan 2019 at 09:10, John McCall via cfe-commits <
>     >>>>> cfe-commits at lists.llvm.org> wrote:
>     >>>>>
>     >>>>>> On 16 Jan 2019, at 9:13, Aaron Ballman wrote:
>     >>>>>>
>     >>>>>>> On Wed, Jan 16, 2019 at 1:57 AM Akira Hatanaka
>     >>>>>>> <ahatanaka at apple.com>
>     >>>>>>> wrote:
>     >>>>>>>>
>     >>>>>>>> Yes, the behavior of the compiler doesn’t match what’s
>     >>>>>>>> explained
>     >>>>>>>> in the documentation anymore.
>     >>>>>>>>
>     >>>>>>>> Please take a look at the attached patch, which updates the
>     >>>>>>>> documentation.
>     >>>>>>>
>     >>>>>>> Patch mostly LGTM, but I did have one wording suggestion.
>     >>>>>>>
>     >>>>>>>> diff --git a/include/clang/Basic/AttrDocs.td
>     >>>>>>>> b/include/clang/Basic/AttrDocs.td
>     >>>>>>>> index 5773a92c9c..ca3cfcf9b2 100644
>     >>>>>>>> --- a/include/clang/Basic/AttrDocs.td
>     >>>>>>>> +++ b/include/clang/Basic/AttrDocs.td
>     >>>>>>>> @@ -2478,15 +2478,20 @@ def TrivialABIDocs : Documentation {
>     >>>>>>>>   let Category = DocCatVariable;
>     >>>>>>>>   let Content = [{
>     >>>>>>>> The ``trivial_abi`` attribute can be applied to a C++ class,
>     >>>>>>>> struct,
>     >>>>>>>> or union.
>     >>>>>>>> -It instructs the compiler to pass and return the type using
>     >>>>>>>> the C
>     >>>>>>>> ABI for the
>     >>>>>>>> +``trivial_abi`` has the following effects:
>     >>>>>>>> +
>     >>>>>>>> +- It instructs the compiler to pass and return the type using
>     >>>>>>>> the C
>     >>>>>>>> ABI for the
>     >>>>>>>> underlying type when the type would otherwise be considered
>     >>>>>>>> non-trivial for the
>     >>>>>>>> purpose of calls.
>     >>>>>>>> -A class annotated with `trivial_abi` can have non-trivial
>     >>>>>>>> destructors or copy/move constructors without automatically
>     >>>>>>>> becoming
>     >>>>>>>> non-trivial for the purposes of calls. For example:
>     >>>>>>>> +- It makes the destructor and copy and move constructors of
>     >>>>>>>> the
>     >>>>>>>> class trivial
>     >>>>>>>> +that would otherwise be considered non-trivial under the C++
>     >>>>>>>> ABI
>     >>>>>>>> rules.
>     >>>>>>>
>     >>>>>>> How about: It makes the destructor, copy constructors, and move
>     >>>>>>> constructors of the class trivial even if they would otherwise
>     >>>>>>> be
>     >>>>>>> non-trivial under the C++ ABI rules.
>     >>>>>>
>     >>>>>> Let's not say that it makes them trivial, because it doesn't.  It
>     >>>>>> causes
>     >>>>>> their immediate non-triviality to be ignored for the purposes of
>     >>>>>> deciding
>     >>>>>> whether the type can be passed in registers.
>     >>>>>>
>     >>>>>
>     >>>>> Given the attribute now forces the type to be passed in registers,
>     >>>>> I
>     >>>> think
>     >>>>> it'd be more to the point to say that it makes the triviality of
>     >>>>> those
>     >>>>> special members be ignored when deciding whether to pass a type
>     >>>>> with a
>     >>>>> subobject of this type in registers.
>     >>>>
>     >>>> Wait, it forces it to be passed in registers?  I thought the design
>     >>>> was that it didn't override the non-trivial-abi-ness of subobjects;
>     >>>> see all the discussion of trivially_relocatable.
>     >>>>
>     >>>
>     >>> The attribute is ill-formed if applied to a class that has a
>     >>> subobject that
>     >>> can't be passed in registers (or that has a vptr). And then as a
>     >>> weird
>     >>> special case we don't instantiate the attribute when instantiating a
>     >>> class
>     >>> if it would be ill-formed (well, we instantiate it and then remove
>     >>> it
>     >>> again, but the effect is the same).
>     >>>
>     >>> The commit at the start of this email chain switches us from the
>     >>> "just
>     >>> override the trivialness for the purposes of the ABI" model to
>     >>> /also/
>     >>> forcing the type to be passed in registers (the type would otherwise
>     >>> not be
>     >>> passed in registers in some corner cases, such as if all its
>     >>> copy/move
>     >>> special members are deleted).
>     >>
>     >> I see.  Alright, I accept your wording, then.
>     >>
>     >> John.
>     >> _______________________________________________
>     >> cfe-commits mailing list
>     >> cfe-commits at lists.llvm.org <mailto:cfe-commits at lists.llvm.org>
>     >> https://urldefense.proofpoint.com/v2/url?u=http-3A__lists.llvm.org_cgi-2Dbin_mailman_listinfo_cfe-2Dcommits&d=DwIGaQ&c=5VD0RTtNlTh3ycd41b3MUw&r=o3kDXzdBUE3ljQXKeTWOMw&m=ywn0RCUoTpZm8YOV2ffRUUVgMx3xapVXDF-yihR7ycI&s=P5RqazYFOIlJWDGViplbmVcGCnxco2SFRE8jbjEiVIY&e=
>     >> <https://urldefense.proofpoint.com/v2/url?u=http-3A__lists.llvm.org_cgi-2Dbin_mailman_listinfo_cfe-2Dcommits&d=DwIGaQ&c=5VD0RTtNlTh3ycd41b3MUw&r=o3kDXzdBUE3ljQXKeTWOMw&m=ywn0RCUoTpZm8YOV2ffRUUVgMx3xapVXDF-yihR7ycI&s=P5RqazYFOIlJWDGViplbmVcGCnxco2SFRE8jbjEiVIY&e=>
>
>
>     _______________________________________________
>     cfe-commits mailing list
>     cfe-commits at lists.llvm.org
>     https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.llvm.org_cgi-2Dbin_mailman_listinfo_cfe-2Dcommits&d=DwIGaQ&c=5VD0RTtNlTh3ycd41b3MUw&r=o3kDXzdBUE3ljQXKeTWOMw&m=ywn0RCUoTpZm8YOV2ffRUUVgMx3xapVXDF-yihR7ycI&s=RqlQW1jluoVzRIWOqcAMcLk2-6YMFlflJblR_OplhVw&e=
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


More information about the cfe-commits mailing list