[PATCH 1/2] Come up with function_decl_type and use it in tree_function_decl.

[Marc Glisse]

On Tue, 9 Jul 2019, Martin Liška wrote:

> On 7/9/19 9:49 AM, Marc Glisse wrote:
>> On Tue, 9 Jul 2019, Marc Glisse wrote:
>>
>>> On Mon, 8 Jul 2019, Martin Liška wrote:
>>>
>>>>> The patch apparently has DECL_IS_OPERATOR_DELETE only on the replaceable global deallocation functions, not all delete operators, contrary to DECL_IS_OPERATOR_NEW, so the name is misleading. On the other hand, those seem to be the ones for which the optimization is legal (well, not quite, the rules are in terms of operator new, and I am not sure how well operator delete has to match, but close enough).
>>>>
>>>> Are you talking about this location where we set OPERATOR_NEW:
>>>> https://github.com/gcc-mirror/gcc/blob/master/gcc/cp/decl.c#L13643
>>>> ?
>>>>
>>>> That's the only place where we set OPERATOR_NEW flag and not OPERATOR_DELETE.
>>>
>>> Yes, I think that's the place.
>>>
>>> Again, not setting DECL_IS_OPERATOR_DELETE on local operator delete
>>> seems misleading, but setting it would let us optimize in cases where we
>>> are not really allowed to. Maybe just rename your macro to
>>> DECL_IS_GLOBAL_OPERATOR_DELETE?
>>
>> Hmm, I replied too fast.
>>
>> Global operator delete does not seem like a good terminology, the ones marked in the patch would be the usual (=non-placement) replaceable deallocation functions.
>>
>> I cannot find a requirement that operator new and operator delete should match. The rules to omit allocation are stated in terms of which operator new is called, but do not seem to care which operator delete is used. So allocating with the global operator new and deallocating with a class overload of operator delete can be removed, but not the reverse (not sure how they came up with such a rule...). Which means we would need:
>
> Thank you Mark for digging deep in that.
>
>>
>> keep DECL_IS_OPERATOR_NEW for the current uses
>>
>> DECL_IS_REPLACEABLE_OPERATOR_NEW (equivalent to DECL_IS_OPERATOR_NEW && DECL_IS_MALLOC? not exactly but close I think) for DCE
>>
>> DECL_IS_OPERATOR_DELETE (which also includes some class overloads) for DCE
>
> Note that with the current version of the patch we are out of free bits in struct GTY(()) tree_function_decl.
> Would it be possible to tweak the current patch to cover what you described?

If you approximate DECL_IS_REPLACEABLE_OPERATOR_NEW with 
DECL_IS_OPERATOR_NEW && DECL_IS_MALLOC, it shouldn't need more bits than 
the current patch. I think the main difference is if a user adds attribute 
malloc to his class-specific operator new, where it will enable DCE, but 
since the attribute is non-standard, we can just document that behavior, 
it might even be desirable.

I am not very confident about anything I said in this thread, for all I 
know I may be misguiding you, please make sure someone else who 
understands the C++ standard approves your final patch.

-- 
Marc Glisse