On 7/9/19 6:17 AM, Marc Glisse wrote:
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...).
Correct. The standard just says that an implementation is allowed to
omit a call to the replaceable ::operator new; it does not place any
constraints on that, the conditions for such omission are left up to the
implementation.
If the user's code uses global new and class delete for the same
pointer, that would suggest that they're doing something odd, and we
might as well leave it alone. I would expect this to be very rare.
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.
Sure, it seems desirable to me.
Jason
