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

Reply via email to