r350920 - [Sema] Make canPassInRegisters return true if the CXXRecordDecl passed
Shoaib Meenai via cfe-commits
cfe-commits at lists.llvm.org
Thu Jan 31 16:37:00 PST 2019
+Hans for managing the 8.0 branch.
On 1/31/19, 4:15 PM, "ahatanaka at apple.com on behalf of Akira Hatanaka" <ahatanaka at apple.com> wrote:
Reverted patch in r352822. I’ll send a new patch later that disallows trivial_abi on classes without non-deleted copy or move constructors.
> On Jan 31, 2019, at 3:52 PM, Richard Smith <richard at metafoo.co.uk> wrote:
>
> 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://urldefense.proofpoint.com/v2/url?u=https-3A__lists.llvm.org_cgi-2Dbin_mailman_listinfo_cfe-2Dcommits&d=DwIFaQ&c=5VD0RTtNlTh3ycd41b3MUw&r=o3kDXzdBUE3ljQXKeTWOMw&m=g5EPqCUh5saO6axHZfXGpX8kSZZLUbixkcCIR9Lcc9o&s=NhJSDmoJK2gLjExO78nxiJS_w5UwzGvCIuEO4zaUVRI&e=
More information about the cfe-commits
mailing list