[Bug c++/34949] Dead code in empty destructors.

[Bug c++/34949] Dead code in empty destructors.

[pinskia at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=34949

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jyasskin at gcc dot gnu.org

--- Comment #7 from Andrew Pinski <pinskia at gcc dot gnu.org> 2011-02-09 22:24:20 UTC ---
*** Bug 47671 has been marked as a duplicate of this bug. ***

[Bug c++/34949] Dead code in empty destructors.

[congruwer at yahoo dot co.uk]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=34949

congruwer at yahoo dot co.uk changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |congruwer at yahoo dot
                   |                            |co.uk

--- Comment #8 from congruwer at yahoo dot co.uk 2011-09-01 08:12:49 UTC ---
I have seen a very similar problem in empty constructors.

[Bug c++/34949] Dead code in empty destructors.

[rguenth at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=34949

--- Comment #9 from Richard Guenther <rguenth at gcc dot gnu.org> 2011-09-01 09:10:11 UTC ---
It looks to me that the C++ FE could detect that the vtable pointer update
would adjust it to itself (I even can't construct a testcase that would
adjust it _not_ to itself ...).

[Bug c++/34949] Dead code in empty destructors.

[jakub at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=34949

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jakub at gcc dot gnu.org

--- Comment #10 from Jakub Jelinek <jakub at gcc dot gnu.org> 2011-09-01 09:31:54 UTC ---
Of course the C++ FE adjusts it to its vtable, it is needed if the body
of the dtor or anything it calls needs to use it.
Andrew's suggestion IMHO makes sense, perhaps using Micha's proposal to say
that
at the end of a destructor *this = volatile{};
Then DSE could happily nuke the vtable ptr stores (or any other stores to the
object being destructed).

[Bug c++/34949] Dead code in empty destructors.

[jason at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=34949

--- Comment #11 from Jason Merrill <jason at gcc dot gnu.org> 2013-03-26 15:14:30 UTC ---
Created attachment 29731
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=29731
patch to add clobbers

This patch adds the clobber assignments as Jakub suggested, but the back end
isn't prepared for clobbers of things other than VAR_DECLs, so the test ICEs in
gimplification.  More back end work is needed.

[Bug c++/34949] Dead code in empty destructors.

[jason at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=34949

Jason Merrill <jason at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #29731|0                           |1
        is obsolete|                            |

--- Comment #12 from Jason Merrill <jason at gcc dot gnu.org> 2013-03-26 15:20:10 UTC ---
Created attachment 29732
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=29732
clobber the whole object

Actually, I guess he was suggesting clobbering all of *this, not just the
vptrs, which makes sense since the object is dead at the end of its destructor.
 This patch still needs more back end work to be useful.

[Bug c++/34949] Dead code in empty destructors.

[jakub at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=34949

--- Comment #13 from Jakub Jelinek <jakub at gcc dot gnu.org> 2013-03-28 14:07:42 UTC ---
Created attachment 29745
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=29745
gcc49-pr34949-1.patch

Incremental patch, passed bootstrap together with the later jason's patch plus
another one I'm going to attach.  There are some testsuite regressions (a
couple of them in libgomp testsuite, couple of them in libstdc++ testsuite) to
be looked at though.

[Bug c++/34949] Dead code in empty destructors.

[jakub at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=34949

--- Comment #14 from Jakub Jelinek <jakub at gcc dot gnu.org> 2013-03-28 14:09:01 UTC ---
Created attachment 29746
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=29746
gcc49-pr34949-2.patch

Another needed patch, without which DSE wouldn't really delete the unneeded
stores.

[Bug c++/34949] Dead code in empty destructors.

[paolo.carlini at oracle dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=34949

--- Comment #15 from Paolo Carlini <paolo.carlini at oracle dot com> 2013-03-28 16:11:31 UTC ---
Cool, thanks guys for working on this. I hope the library issues aren't too
serious.

[Bug c++/34949] Dead code in empty destructors.

[paolo.carlini at oracle dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=34949

--- Comment #17 from Paolo Carlini <paolo.carlini at oracle dot com> 2013-03-28 23:58:04 UTC ---
I see.

[Bug c++/34949] Dead code in empty destructors.

[jason at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=34949

Jason Merrill <jason at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
                 CC|                            |jason at gcc dot gnu.org
         Resolution|                            |FIXED
   Target Milestone|---                         |4.9.0

--- Comment #18 from Jason Merrill <jason at gcc dot gnu.org> 2013-04-02 21:10:55 UTC ---
Fixed for 4.9.

[Bug c++/34949] Dead code in empty destructors.

[jakub at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=34949

--- Comment #21 from Jakub Jelinek <jakub at gcc dot gnu.org> 2013-04-03 06:31:50 UTC ---
Author: jason
Date: Tue Apr  2 21:09:38 2013
New Revision: 197375

URL: http://gcc.gnu.org/viewcvs?rev=197375&root=gcc&view=rev
Log:
    PR c++/34949
    * decl.c (begin_destructor_body): Clobber the object in a cleanup.

Added:
    trunk/gcc/testsuite/g++.dg/opt/vt2.C
Modified:
    trunk/gcc/cp/ChangeLog
    trunk/gcc/cp/decl.c

[Bug c++/34949] Dead code in empty destructors.

[jakub at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=34949

--- Comment #19 from Jakub Jelinek <jakub at gcc dot gnu.org> 2013-04-03 06:31:02 UTC ---
Author: jakub
Date: Tue Apr  2 18:25:36 2013
New Revision: 197369

URL: http://gcc.gnu.org/viewcvs?rev=197369&root=gcc&view=rev
Log:
    PR c++/34949
    * tree-cfg.c (verify_gimple_assign_single): Allow lhs
    of gimple_clobber_p to be MEM_REF.
    * gimplify.c (gimplify_modify_expr): Gimplify *to_p of
    an assignment from TREE_CLOBBER_P.  Allow it to be MEM_REF
    after gimplification.
    * asan.c (get_mem_ref_of_assignment): Don't instrument
    gimple_clobber_p stmts.
    * tree-ssa-dse.c (dse_optimize_stmt): Allow DSE of
    gimple_clobber_p stmt if they have MEM_REF lhs and
    are dead because of another gimple_clobber_p stmt.
    * tree-ssa-live.c (clear_unused_block_pointer): Treat
    gimple_clobber_p stmts like debug stmts.
    (remove_unused_locals): Remove clobbers with MEM_REF lhs
    that refer to unused VAR_DECLs or uninitialized values.
    * tree-sra.c (sra_ipa_reset_debug_stmts): Also remove
    gimple_clobber_p stmts if they refer to removed parameters.
    (get_repl_default_def_ssa_name, sra_ipa_modify_expr): Fix up
    formatting.

Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/asan.c
    trunk/gcc/gimplify.c
    trunk/gcc/tree-cfg.c
    trunk/gcc/tree-sra.c
    trunk/gcc/tree-ssa-dse.c
    trunk/gcc/tree-ssa-live.c

[Bug c++/34949] Dead code in empty destructors.

[jakub at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=34949

--- Comment #20 from Jakub Jelinek <jakub at gcc dot gnu.org> 2013-04-03 06:31:13 UTC ---
Author: jakub
Date: Tue Apr  2 18:27:45 2013
New Revision: 197370

URL: http://gcc.gnu.org/viewcvs?rev=197370&root=gcc&view=rev
Log:
    PR c++/34949
    * tree-ssa-alias.c (stmt_kills_ref_p_1): If base != ref->base
    and both of them are MEM_REFs, just compare first argument for
    equality and attempt to deal even with differing offsets.

Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/tree-ssa-alias.c

[Bug c++/34949] Dead code in empty destructors.

[schwab@linux-m68k.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=34949

--- Comment #22 from Andreas Schwab <schwab@linux-m68k.org> 2013-04-03 09:27:28 UTC ---
That breaks g++.dg/tree-ssa/ehcleanup-1.C

$ grep Removing ehcleanup-1.C.032t.ehcleanup1 
Removing unreachable region 1
Removing unreachable region 2
Removing unreachable region 3
Removing unreachable region 4
Removing basic block 5
Removing basic block 4
Removing unreachable region 1
Removing unreachable region 2

[Bug c++/34949] Dead code in empty destructors.

[jakub at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=34949

--- Comment #23 from Jakub Jelinek <jakub at gcc dot gnu.org> 2013-04-03 09:39:34 UTC ---
(In reply to comment #22)
> That breaks g++.dg/tree-ssa/ehcleanup-1.C

On which target?  I saw no regressions on x86_64-linux and i686-linux with all
the 3 patches.

[Bug c++/34949] Dead code in empty destructors.

[jakub at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=34949

--- Comment #25 from Jakub Jelinek <jakub at gcc dot gnu.org> 2013-04-03 12:27:56 UTC ---
Looking at it more, if the change to emit *this ={v} {CLOBBER}; in cleanups
rather than just at the end of function was desirable, then IMHO we just want
to adjust the ehcleanup1.C testcase (from 4 to 6), those 2 removed regions are
in a::~a where previously nothing has been cleaned up.

[Bug c++/34949] Dead code in empty destructors.

[jason at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=34949

--- Comment #26 from Jason Merrill <jason at gcc dot gnu.org> 2013-04-03 13:37:51 UTC ---
(In reply to comment #25)
> Looking at it more, if the change to emit *this ={v} {CLOBBER}; in cleanups
> rather than just at the end of function was desirable, then IMHO we just want
> to adjust the ehcleanup1.C testcase (from 4 to 6), those 2 removed regions are
> in a::~a where previously nothing has been cleaned up.

The change was desirable so that we can do DSE on the EH path as well as normal
control flow, as needed for 50243.  I've adjusted the testcase.

[Bug c++/34949] Dead code in empty destructors.

[izamyatin at gmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=34949

Igor Zamyatin <izamyatin at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |izamyatin at gmail dot com

--- Comment #27 from Igor Zamyatin <izamyatin at gmail dot com> 2013-04-04 07:18:07 UTC ---
Still, I see that r197375 breaks spec2006/483.xalancbmk compilation:

g++ -mavx  -static -c -o DOMDocumentImpl.o -DSPEC_CPU -DNDEBUG 
-DAPP_NO_THREADS -DXALAN_INMEM_MSG_LOADER -I. -Ixercesc -Ixercesc/dom
-Ixercesc/dom/impl -Ixercesc/sax -Ixercesc/util/MsgLoaders/InMemory
-Ixercesc/util/Transcoders/Iconv -Ixalanc/include -DPROJ_XMLPARSER
-DPROJ_XMLUTIL -DPROJ_PARSERS -DPROJ_SAX4C -DPROJ_SAX2 -DPROJ_DOM
-DPROJ_VALIDATORS -DXML_USE_NATIVE_TRANSCODER -DXML_USE_INMEM_MESSAGELOADER -O3
-funroll-loops -ffast-math -march=corei7   -DSPEC_CPU_LP64  -DSPEC_CPU_LINUX   
 DOMDocumentImpl.cpp

DOMDocumentImpl.cpp: In destructor
'xercesc_2_5::DOMDocumentImpl::~DOMDocumentImpl()':
DOMDocumentImpl.cpp:207:1: error: definition in block 47 does not dominate use
in block 92
 DOMDocumentImpl::~DOMDocumentImpl()
 ^
for SSA_NAME: _36 in statement:
# .MEM_250 = VDEF <.MEM_8>
MEM[(struct BaseRefVectorOf *)_36 + 8B] ={v} {CLOBBER};
DOMDocumentImpl.cpp:207:1: internal compiler error: verify_ssa failed
0xc06b54 verify_ssa(bool)
        ../../gcc/tree-ssa.c:1046
0x9e6662 execute_function_todo
        ../../gcc/passes.c:1964
0x9e704d execute_todo
        ../../gcc/passes.c:1996

[Bug c++/34949] Dead code in empty destructors.

[jakub at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=34949

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|RESOLVED                    |REOPENED
         Resolution|FIXED                       |

--- Comment #28 from Jakub Jelinek <jakub at gcc dot gnu.org> 2013-04-04 07:31:58 UTC ---
Yeah, I can reproduce, reducing now.

[Bug c++/34949] Dead code in empty destructors.

[jakub at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=34949

--- Comment #29 from Jakub Jelinek <jakub at gcc dot gnu.org> 2013-04-04 08:25:08 UTC ---
Reduced testcase for -O3:

struct E {};
struct A
{
  virtual void a (void *) = 0;
};
struct B
{
  virtual ~B () {};
  unsigned int b1;
  E **b2;
  A *b3;
};
struct C : public B
{
  ~C ();
};
C::~C ()
{
  for (unsigned int i = 0; i < b1; i++)
    b3->a (b2);
}
struct D
{
  ~D () {}
  C d;
};
struct F { virtual ~F () {}; };
struct G { void g (); };
struct H : public F
{
  virtual ~H ();
  D *h1;
  G *h2;
};
H::~H ()
{
  h2->g ();
  delete h1;
}

[Bug c++/34949] Dead code in empty destructors.

[jakub at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=34949

--- Comment #30 from Jakub Jelinek <jakub at gcc dot gnu.org> 2013-04-04 09:20:36 UTC ---
Ah, I guess it is sink_clobbers, which would need to be tought to drop MEM_REF
clobbers instead of sinking them if the target bb is no longer dominated by the
bb with the SSA_NAME definition.

[Bug c++/34949] Dead code in empty destructors.

[jakub at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=34949

--- Comment #31 from Jakub Jelinek <jakub at gcc dot gnu.org> 2013-04-04 12:52:36 UTC ---
Created attachment 29803
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=29803
gcc49-pr34949.patch

Untested fix (and enhancement too).

[Bug c++/34949] Dead code in empty destructors.

[jakub at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=34949

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|REOPENED                    |RESOLVED
         Resolution|                            |FIXED

--- Comment #32 from Jakub Jelinek <jakub at gcc dot gnu.org> 2013-04-08 13:50:42 UTC ---
Author: jakub
Date: Mon Apr  8 13:46:00 2013
New Revision: 197580

URL: http://gcc.gnu.org/viewcvs?rev=197580&root=gcc&view=rev
Log:
    PR c++/34949
    PR c++/50243
    * tree-eh.c (optimize_clobbers): Only remove clobbers if bb doesn't
    contain anything but clobbers, at most one __builtin_stack_restore,
    optionally debug stmts and final resx, and if it has at least one
    incoming EH edge.  Don't check for SSA_NAME on LHS of a clobber.
    (sink_clobbers): Don't check for SSA_NAME on LHS of a clobber.
    Instead of moving clobbers with MEM_REF LHS with SSA_NAME address
    which isn't defaut definition, remove them.
    (unsplit_eh, cleanup_empty_eh): Use single_{pred,succ}_{p,edge}
    instead of EDGE_COUNT comparisons or EDGE_{PRED,SUCC}.
    * tree-ssa-ccp.c (execute_fold_all_builtins): Remove clobbers
    with MEM_REF LHS with SSA_NAME address.

    * g++.dg/opt/vt3.C: New test.
    * g++.dg/opt/vt4.C: New test.

Added:
    trunk/gcc/testsuite/g++.dg/opt/vt3.C
    trunk/gcc/testsuite/g++.dg/opt/vt4.C
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/tree-eh.c
    trunk/gcc/tree-ssa-ccp.c

[Bug c++/34949] Dead code in empty destructors.

[dimhen at gmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=34949

--- Comment #33 from Dmitry G. Dyachenko <dimhen at gmail dot com> 2013-04-09 10:07:47 UTC ---
*** Bug 56830 has been marked as a duplicate of this bug. ***

[Bug c++/34949] Dead code in empty destructors.

[pinskia at gcc dot gnu dot org]

------- Comment #6 from pinskia at gcc dot gnu dot org  2009-06-09 10:18 -------
*** Bug 40382 has been marked as a duplicate of this bug. ***


-- 

pinskia at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |carrot at google dot com


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=34949

[Bug c++/34949] Dead code in empty destructors.

[pinskia at gcc dot gnu dot org]

------- Comment #5 from pinskia at gcc dot gnu dot org  2008-01-24 18:39 -------
This still needs communication from the front-end to the middle-end.  The
middle-end part should be a rather easy extension to both the RTL and tree
level DSEs really.

The way I see fixing it is adding a statement which marks *this dead at some
point.  This might be used for fixing PR 19831 also. 


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=34949

[Bug c++/34949] Dead code in empty destructors.

[spark at gcc dot gnu dot org]

------- Comment #4 from spark at gcc dot gnu dot org  2008-01-24 18:16 -------
I don't think this can be implemented in the FE,
at least to be really effective.
The more common cases are not really "empty" destructor.
E.g. there's inlining involved:

class Foo
{
public:
  virtual ~Foo();
};

Foo::~Foo()
{
}


class Bar : public Foo
{
public:
  virtual ~Bar();
};

Bar::~Bar()
{
}


where Foo::~Foo() has to be inlined before we can determine
~Bar() is really empty.

But "emptiness" isn't really how we want to decide eliminating those
but rather the dependence on the vtable itself. e.g.:
extern "C" int printf(const char* s);

class Foo
{
public:
  virtual ~Foo();
};

Foo::~Foo()
{
  printf("good bye foo\n");
}


class Bar : public Foo
{
public:
  virtual ~Bar();
};

Bar::~Bar()
{
  printf("good bye bar\n");
}

In this case, there's no need for vtable update inside the destructor
(which is more common).


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=34949

[Bug c++/34949] Dead code in empty destructors.

[rguenth at gcc dot gnu dot org]

------- Comment #3 from rguenth at gcc dot gnu dot org  2008-01-24 10:32 -------
This needs to be optimized in the C++ FE for the said reasons.


-- 

rguenth at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Severity|normal                      |enhancement
           Keywords|                            |missed-optimization


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=34949

[Bug c++/34949] Dead code in empty destructors.

[spark at gcc dot gnu dot org]

------- Comment #2 from spark at gcc dot gnu dot org  2008-01-23 23:23 -------
I understand why it's difficult to delete the code in the middle-end. But this
case seems to be quite common - not the empty body but more common cases are
destructors whose body doesn't need vtable.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=34949

[Bug c++/34949] Dead code in empty destructors.

[pinskia at gcc dot gnu dot org]

------- Comment #1 from pinskia at gcc dot gnu dot org  2008-01-23 23:17 -------
hmmm, it is hard for middle-end to know these are dead, the C++ front-end needs
to comuniticate to the middle-end that after the function ends, this is not
going to be valid.

By the way, please don't call this a trivial destructor as it is not truely
trivial as defined by the C++ standard; trivial as defined by the C++ standard
means it does not exist and is compiler generated.


-- 

pinskia at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
          Component|middle-end                  |c++
     Ever Confirmed|0                           |1
   Last reconfirmed|0000-00-00 00:00:00         |2008-01-23 23:17:07
               date|                            |
            Summary|Dead code in trivial        |Dead code in empty
                   |destructors.                |destructors.


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=34949