On Tue, Aug 6, 2019 at 2:42 PM Martin Liška <[email protected]> wrote:
>
> 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.
I think the simplest way to achieve this is to not copy, aka clear,
DECL_IS_OPERATOR_* when cloning and removing arguments
(cloning for a constant align argument should be OK for example, as is
for a constant address). Or simply always when cloning.
Richard.
> >
> > 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...
> >
>
