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

[Shoaib Meenai via cfe-commits]

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=