[PATCH] Check DECL_CONTEXT of new/delete operators.

[Richard Biener]

On Tue, Apr 7, 2020 at 5:17 PM Jonathan Wakely <jwakely.gcc@gmail.com> wrote:
>
> On Tue, 7 Apr 2020 at 12:57, Richard Biener <richard.guenther@gmail.com> wrote:
> >
> > On Tue, Apr 7, 2020 at 1:46 PM Jonathan Wakely <jwakely.gcc@gmail.com> wrote:
> > >
> > > On Tue, 7 Apr 2020 at 12:40, Richard Biener <richard.guenther@gmail.com> wrote:
> > > >
> > > > On Tue, Apr 7, 2020 at 1:30 PM Jonathan Wakely <jwakely.gcc@gmail.com> wrote:
> > > > >
> > > > > On Mon, 6 Apr 2020 at 13:45, Nathan Sidwell wrote:
> > > > > > The both operator new and operator delete are looked up in the same
> > > > > > manner.  The std does not require a 'matching pair' be found.  but it is
> > > > > > extremely poor form for a class to declare exactly one of operator
> > > > > > {new,delete}.
> > > > >
> > > > > There are unfortunately several such example in the standard!
> > > > >
> > > > > I wonder how much benefit we will really get from trying to make this
> > > > > optimisation too general.
> > > > >
> > > > > Just eliding (or coalescing) implicit calls to ::operator new(size_t)
> > > > > and ::operator delete(void*, size_t) (and the [] and align_val_t forms
> > > > > of those) probably covers 99% of real cases.
> > > >
> > > > IIRC the only reason to have the optimization was to fully elide
> > > > STL containers when possible.  That brings in allocators and
> > > > thus non ::new/::delete allocations.
> > >
> > > But the vast majority of containers are used with std::allocator, and
> > > we control that.
> > >
> > > Wouldn't it be simpler to add __builtin_operator_new and
> > > __builtin_operator_delete like clang has, then make std::allocator use
> > > those, and limit the optimizations to uses of those built-ins?
> >
> > Sure, that's a viable implementation strathegy.  Another might be
> >
> > void delete (void *) __attribute__((matching_new(somewhere::new)));
> >
> > and thus allow the user to relate a new/delete pair (here the FE would
> > do lookup for 'new' and record for example the mangled assembler name).
>
> Does that attribute tell us anything useful?
>
> Given a pointer obtained from new and passed to delete, can't we
> assume they are a matching pair? If not, the behaviour would be
> undefined anyway.

Further matching of new/delete came up in the context of inlining where
we might not be able to elide side-effects of new/delete "appropriately".
That's actually the case in the referenced PR.

> > That is, we usually try to design a mechanism that's more broadly usable.
> > But yes, we match malloc/free so matching ::new/::delete by aliasing them
> > to __builtin_operator_new and __builtin_operator_delete is fair enough.
>
> Would it make sense to annotate the actual calls to the
> allocation/deallocation functions, instead of the declarations of
> those functions?

Sure - I think that's ultimatively the way to go.

> According to Richard Smith, we don't want to optimise away 'operator
> delete(operator new(1), 1)' because that's an explicit call, and the
> user might have replaced those functions and be relying on the side
> effects. But we can optimise away 'delete new char' and we can
> optimise away std::allocator<char>().deallocate(std::allocator<char>().allocate(1),
> 1). So what matters is not whether the functions being called match up
> (they have to anyway) or which functions are being called. What
> matters is whether they are called implicitly (by a new-expression or
> by std::allocator).

OK, I see.  So for the inline case we'd for example have

operator new() { return ::new ...; }

and

 delete new T;

where the generated and marked operator new is inlined we'd no longer
elide the pair because the operator new implementation had an explicit
call to ::new and this is what the optimization sees now.  Only when
the operator new implementation uses a new expression again we'd
see it as candidate pair and then run into the inline issue again
(the issue of the new operator implementation incrementing some counter
and the delete one decrementing it, us inlining one of both and then
eliding the pair, only eliding either the increment or the decrement
and thus retaining half of the overall side-effect).

> So when the compiler expands 'new T' into a call to operator new
> followed by constructing a T (plus exception handling) the call to
> operator new would be marked as optimisable by the FE, and likewise
> when the compiler expands 'delete p' into a destructor followed by
> calling operator delete, the call to operator delete would be marked
> as optimisable. If a pointer is allocated by a call marked optimisable
> and deallocated by a call marked optimisable, it can be optimised away
> (or coalesced with another optimisation).
>
> Then for std::allocator we just want to be able to mark the explicit
> calls to operator new and operator delete as "optimisable", as the FE
> does for the implicit calls it generates. So if we want a general
> purpose utility, it would be an attribute to mark /calls/ as
> optimisable, not functions.
>
> Adding __builtin_operator_new would just be a different syntax for
> "call operator new but mark it optimisable".

Ah, so __builtin_operator_new isn't a function but an alternate
new expression syntax?

Richard.

> N.B. I am not a compiler dev and might be talking nonsense :-)