Re: [PATCH] Handle new operators with no arguments in DCE.

["Martin Liška" <mliska@suse.cz>]

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...
>