[Bug c++/50243] New: vtable for pure abstract class (interface) shouldn't be emitted

[Bug c++/50243] New: vtable for pure abstract class (interface) shouldn't be emitted

[congruwer at yahoo dot co.uk]

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

             Bug #: 50243
           Summary: vtable for pure abstract class (interface) shouldn't
                    be emitted
    Classification: Unclassified
           Product: gcc
           Version: 4.5.2
            Status: UNCONFIRMED
          Severity: enhancement
          Priority: P3
         Component: c++
        AssignedTo: unassigned@gcc.gnu.org
        ReportedBy: congruwer@yahoo.co.uk


Note: this is probably easier to do if ‘Bug 34949 - Dead code in empty
destructors.’ is done.

Consider the following class:

class iface
{
protected:
    ~iface() { }
public:
    virtual void a() = 0;
    virtual void b() = 0;
    virtual void c() = 0;
};

This class cannot be instantiated, so a, b and c cannot be called from outside.
The only possible call site for them would be the destructor ~iface() but from
the fact that a, b and c are pure and the fact that it compiles, we know that
this doesn't happen. So the vtable for this class shouldn't be emitted.

But it is:

__ZTV5iface:
    .long    0
    .long    0
    .long    ___cxa_pure_virtual
    .long    ___cxa_pure_virtual
    .long    ___cxa_pure_virtual

To make matters worse, it's needlessly referenced in destructors of derived
classes:

__ZN4impl1cEv:
    pushl    %ebx
    subl    $8, %esp
    movl    16(%esp), %ebx
    testl    %ebx, %ebx
    je    L3
    movl    $__ZTV4impl+8, (%ebx)     <--- Strictly speaking unnecessary
    call    __Z7dostuffv
    movl    $__ZTV5iface+8, (%ebx)    <--- OOPS
    movl    %ebx, 16(%esp)              What follows is the inlined
    addl    $8, %esp                    destructor of iface, the
    popl    %ebx                        iface vtable isn't needed.
    jmp    __ZdlPv

For this example I deliberately used a small interface, but I have found that
in larger software projects the unnecessary vtables can add up.

For reference, the rest of code used to demonstrate the problem follows:

void dostuff();

class impl : public iface
{
private:
    ~impl() { dostuff(); }
public:
    void a() { dostuff(); }
    void b() { dostuff(); }
    void c() { delete this; }
};

void test()
{
    iface * y = new impl();
    y->a();
    y->b();
    y->c();
}

[Bug c++/50243] vtable for pure abstract class (interface) shouldn't be emitted

[pinskia at gcc dot gnu.org]

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

--- Comment #1 from Andrew Pinski <pinskia at gcc dot gnu.org> 2011-08-30 20:54:34 UTC ---
The vtable is required by the ABI IIRC.

[Bug c++/50243] vtable for pure abstract class (interface) shouldn't be emitted

[congruwer at yahoo dot co.uk]

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

--- Comment #2 from congruwer at yahoo dot co.uk 2011-08-31 15:27:52 UTC ---
Not in this case. The example is set up such that the vtable is invisible, even
if it is emitted. Since no one can access it, it cannot be required. (I have
searched through the ABI documentation again, and it doesn't seem to require
the presence of unreferenced vtables. And if it did, that would be a logical
contradiction.)
There is, in principle, an absolute guarantee that the fields of iface's vtable
will never get accessed, and that ___cxa_pure_virtual will never get called, or
at least not through iface's vtable.

[Bug c++/50243] vtable for pure abstract class (interface) shouldn't be emitted

[jason at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2013-03-25
                 CC|                            |jason at gcc dot gnu.org
         Depends on|                            |34949
     Ever Confirmed|0                           |1

--- Comment #3 from Jason Merrill <jason at gcc dot gnu.org> 2013-03-25 22:11:45 UTC ---
(In reply to comment #0)
> Note: this is probably easier to do if ‘Bug 34949 - Dead code in empty
> destructors.’ is done.

Yes, this is really a consequence, almost a duplicate, of 34949.  If we
recognize that the store is dead because nothing can refer to the object's
vptr, then we can optimize away the reference to the vtable and so the vtable
won't be emitted.
>From gcc-bugs-return-418402-listarch-gcc-bugs=gcc.gnu.org@gcc.gnu.org Mon Mar 25 22:18:15 2013
Return-Path: <gcc-bugs-return-418402-listarch-gcc-bugs=gcc.gnu.org@gcc.gnu.org>
Delivered-To: listarch-gcc-bugs@gcc.gnu.org
Received: (qmail 2807 invoked by alias); 25 Mar 2013 22:18:15 -0000
Mailing-List: contact gcc-bugs-help@gcc.gnu.org; run by ezmlm
Precedence: bulk
List-Id: <gcc-bugs.gcc.gnu.org>
List-Archive: <http://gcc.gnu.org/ml/gcc-bugs/>
List-Post: <mailto:gcc-bugs@gcc.gnu.org>
List-Help: <mailto:gcc-bugs-help@gcc.gnu.org>
Sender: gcc-bugs-owner@gcc.gnu.org
Delivered-To: mailing list gcc-bugs@gcc.gnu.org
Received: (qmail 2384 invoked by uid 48); 25 Mar 2013 22:18:08 -0000
From: "dnovillo at gcc dot gnu.org" <gcc-bugzilla@gcc.gnu.org>
To: gcc-bugs@gcc.gnu.org
Subject: [Bug bootstrap/54659] [4.8 Regression] Bootstrap with --disable-nls broken under Windows
Date: Mon, 25 Mar 2013 22:18:00 -0000
X-Bugzilla-Reason: CC
X-Bugzilla-Type: changed
X-Bugzilla-Watch-Reason: None
X-Bugzilla-Product: gcc
X-Bugzilla-Component: bootstrap
X-Bugzilla-Keywords: build
X-Bugzilla-Severity: normal
X-Bugzilla-Who: dnovillo at gcc dot gnu.org
X-Bugzilla-Status: REOPENED
X-Bugzilla-Priority: P1
X-Bugzilla-Assigned-To: rguenth at gcc dot gnu.org
X-Bugzilla-Target-Milestone: 4.8.0
X-Bugzilla-Changed-Fields: Status Resolution
Message-ID: <bug-54659-4-kdde0MC6sN@http.gcc.gnu.org/bugzilla/>
In-Reply-To: <bug-54659-4@http.gcc.gnu.org/bugzilla/>
References: <bug-54659-4@http.gcc.gnu.org/bugzilla/>
X-Bugzilla-URL: http://gcc.gnu.org/bugzilla/
Auto-Submitted: auto-generated
Content-Type: text/plain; charset="UTF-8"
MIME-Version: 1.0
X-SW-Source: 2013-03/txt/msg01843.txt.bz2
Content-length: 487


http://gcc.gnu.org/bugzilla/show_bug.cgi?idT659

Diego Novillo <dnovillo at gcc dot gnu.org> changed:

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

--- Comment #16 from Diego Novillo <dnovillo at gcc dot gnu.org> 2013-03-25 22:18:06 UTC ---
Re-opening on request from Roland.

[Bug c++/50243] vtable for pure abstract class (interface) shouldn't be emitted

[jason at gcc dot gnu.org]

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

--- Comment #4 from Jason Merrill <jason at gcc dot gnu.org> 2013-04-02 21:19:18 UTC ---
Created attachment 29782
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=29782
possible patch

The patches we checked in today for 34949 almost fix this bug as well, but not
quite; the vtable pointer assignment remains in the exception handler, because
optimize_clobbers in tree-eh.c is removing the clobber before it can be used to
optimize away the assignment.

This patch fixes that by skipping MEM_REF clobbers, but I suspect this isn't
really the right way to fix the issue.  Jakub?

[Bug c++/50243] vtable for pure abstract class (interface) shouldn't be emitted

[jakub at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |matz at gcc dot gnu.org,
                   |                            |rth at gcc dot gnu.org

--- Comment #5 from Jakub Jelinek <jakub at gcc dot gnu.org> 2013-04-03 07:15:30 UTC ---
I think the patch in this form looks undesirable, we could often generate worse
code (containing many more rethrows than really needed).  If we keep the
clobbers there, it might prevent EH optimizations from happening.
See PR51117 for more details.  So, either we'd need to run DSE before the EH
optimizations, or perhaps we could just make optimize_clobbers smarter.  As in,
look through all the clobbers (+ debug stmts) after the externally throwing
resx, and when we reach the first, check the VUSE of the first clobber.  If the
VUSE's VDEF is a PHI at the beginning of block, then optimize all the clobbers
away, if the VDEF is in a different bb and the path from that other bb to the
current one is through some EH edges, also optimize those away, but if the VDEF
is non-PHI in the same bb, keep the clobber around, and similarly if the VDEF
is in some other bb and there is no EH (or complex of any kind?) edge in
between, keep the clobbers.  If there are some other stores, the current EH
cleanup pass won't be able to optimize anything anyway, so the clobbers there
don't pessimize anything.  On this particular testcase we have:
<L1>:
# .MEM_13 = VDEF <.MEM_4>
MEM[(struct iface *)this_2(D)]._vptr.iface = &MEM[(void *)&_ZTV5iface + 16B];
# .MEM_14 = VDEF <.MEM_13>
MEM[(struct iface *)this_2(D)] ={v} {CLOBBER};
resx 2

<L2>:
# .MEM_10 = VDEF <.MEM_14>
*this_2(D) ={v} {CLOBBER};
resx 1

so I'd say we want to remove the second clobber, as it is desirable the EH
cleanup considers the bb as empty, but keep the first clobber, there is code
before it anyway and it will be useful to attempt to DSE it.

It would be nice if we had some later EH cleanup clean it up if we DSE the
store.  Looking through passes.c, we schedule another pass_cleanup_eh quite
late, so that could be taken care of.  Another pass that calls
optimize_clobbers/sink_clobbers is the lower eh dispatch pass, don't remember
what exact changes does it do to the EH depending on whether there is code
before resx or not, and whether it is something some other pass (cleanup EH?)
is able to optimize later.  If not, we should either teach the cleanup empty eh
pass to optimize it, if e.g. DSE etc. makes those blocks empty (+
optimize_clobbers), or add a new pass that would do that.

[Bug c++/50243] vtable for pure abstract class (interface) shouldn't be emitted

[jakub at gcc dot gnu.org]

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

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

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

--- Comment #6 from Jakub Jelinek <jakub at gcc dot gnu.org> 2013-04-08 13:51:07 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++/50243] vtable for pure abstract class (interface) shouldn't be emitted

[fxcoudert at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=50243

Francois-Xavier Coudert <fxcoudert at gcc dot gnu.org> changed:

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

--- Comment #7 from Francois-Xavier Coudert <fxcoudert at gcc dot gnu.org> ---
(In reply to Jakub Jelinek from comment #6)
> Author: jakub
> Date: Mon Apr  8 13:46:00 2013
> New Revision: 197580

It appears that the fix doesn't work for darwin:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=56906