On 8/5/19 3:46 PM, Marc Glisse wrote: > On Mon, 5 Aug 2019, Martin Liška wrote: > >> You are right. It can really lead to confusion of the DCE. >> >> What we have is DECL_ABSTRACT_ORIGIN(decl) which we can use to indicate >> operators >> that were somehow modified by an IPA optimization. > > Looks similar to the cgraph_node->clone_of that Richard was talking about. I > have no idea which one would be best.
Hm, strange that the ISRA clones don't have n->clone_of set. It's created here: #0 cgraph_node::create (decl=<function_decl 0x7ffff721c300 _ZN1AdlEPvd.isra.0>) at /home/marxin/Programming/gcc/gcc/profile-count.h:751 #1 0x0000000000bc8342 in cgraph_node::create_version_clone (this=<cgraph_node * const 0x7ffff7208000 "operator delete"/64>, new_decl=<optimized out>, redirect_callers=..., bbs_to_copy=0x0, suffix=0x1b74427 "isra") at /home/marxin/Programming/gcc/gcc/cgraphclones.c:960 #2 0x0000000000bc9b9a in cgraph_node::create_version_clone_with_body (this=this@entry=<cgraph_node * const 0x7ffff7208000 "operator delete"/64>, redirect_callers=redirect_callers@entry=..., tree_map=tree_map@entry=0x0, args_to_skip=args_to_skip@entry=0x0, skip_return=skip_return@entry=false, bbs_to_copy=bbs_to_copy@entry=0x0, new_entry_block=<optimized out>, suffix=<optimized out>, target_attributes=<optimized out>) at /home/marxin/Programming/gcc/gcc/cgraphclones.c:1071 #3 0x00000000010e4611 in modify_function (adjustments=..., node=<cgraph_node * 0x7ffff7208000 "operator delete"/64>) at /home/marxin/Programming/gcc/gcc/tree-sra.c:5493 #4 ipa_early_sra () at /home/marxin/Programming/gcc/gcc/tree-sra.c:5731 #5 (anonymous namespace)::pass_early_ipa_sra::execute (this=<optimized out>) at /home/marxin/Programming/gcc/gcc/tree-sra.c:5778 @Martin, @Honza: Why do we not set clone_of in this transformation? > >> Do you believe it will be sufficient? > > In DCE only consider the operator delete that are not clones? (possibly the > same for operator new? I don't know how much we can change the return value > of a function in a clone) I guess that should work, and it wouldn't impact > the common case with default, global operator new/delete. I tent to limit that the only cgraph nodes that are not clones. I'm going to prepare a patch for it. > > An alternative would be to clear the DECL_IS_OPERATOR_DELETE flag when > creating a clone (possibly only if it modified the first parameter?). There > is probably not much information in the fact that a function that doesn't > even take a pointer used to be an operator delete. > > > By the way, I just thought of something, now that we handle class-specific > operator new/delete (but you could do the same with the global replacable > ones as well): > > #include <stdio.h> > int count = 0; > struct A { > __attribute__((malloc,noinline)) > static void* operator new(unsigned long sz){++count;return ::operator > new(sz);} > static void operator delete(void* ptr){--count;::operator delete(ptr);} > }; > int main(){ > delete new A; > printf("%d\n",count); > } > > If we do not inline anything, we can remove the pair and nothing touches > count. If we inline both new and delete, we can then remove the inner pair > instead, count increases and decreases, fine. If we inline only one of them, > and DCE the mismatched pair new/delete, we get something inconsistent (count > is -1). > > This seems to indicate we should check that the new and delete match > somehow... >