On 4/7/20 12:22 PM, Richard Biener wrote:
On Tue, Apr 7, 2020 at 11:49 AM Jan Hubicka <hubi...@ucw.cz> wrote:
Well, the question is how to identify "operator new and operator delete at the
point of the new-expression and delete-expression". Currently we're
just matching up "is this a new/delete operator" and the dataflow of the
pointer. In the PR it was suggested the actual called methods should have
the same DECL_CONTEXT. Honza suggested the context should have the
"same" ODR type (or be global).
I was just arguing that comparing pointers to types does not make much
sence in LTO where types can get unshared :)
Since the classes have ODR name at least this problem can be solved by
using ODR name compare.
Sure.
However the testcase I sent abuses the fact that if you inherit the
class you can rewrite only new operator. In the inerited class
DECL_CONTEXT of delete will still point to the oriignal class. This
means that you can mix new/delete pair from the original and inherited
class.
Yeah, but we're searching for a correctness fix not for an optimality one ;)
It sounds matching up the pairs in the middle-end is impossible and thus
the FE has to do this match-up (or refrain from marking new/delete when
a matchup according to to be defined methods is not going to be enough).
And maybe that tracking has to be done on a per call level rather than
on a called function level.
Based on what was said here, I see a proper matching implementation quite
expensive
from implementation point of point. Moreover, the optimization itself has quite
low impact and so I suggest to only remove matching replaceable operators (1-12)
from [1], which are the top-level ones.
I'll come up with DECL_IS_REPLACEABLE_OPERATOR_DELETE_P and fix
#define DECL_IS_REPLACEABLE_OPERATOR_NEW_P(NODE) \
(DECL_IS_OPERATOR_NEW_P (NODE) && DECL_IS_MALLOC (NODE))
which is not correct. It also matches
struct A
{
__attribute__((noinline))
static void* operator new(unsigned long sz)
{
++count;
return ::operator new(sz);
}
where DECL_IS_MALLOC is discovered by local IPA passes.
Hope it's viable solution?
Thanks,
Martin
[1] https://en.cppreference.com/w/cpp/memory/new/operator_delete
Richard.
Honza
You make it sound it's much harder and the parser needs to build the
relation? You also suggest the "validness" is only OK in the context
of std::allocator and based on the unspecifiedness of the number of
allocations from the standard library. That would further suggest that
we need to mark the allocation/deallocation points somehow and _not_
base the optimization on the called new/delete "function" (maybe with
an exception of the global ::new and ::delete).
Richard.
