diff mbox

C++ PATCH for lto/53808 (devirtualization of defaulted virtual dtor)

Message ID 530CE6A1.2010003@redhat.com
State New
Headers show

Commit Message

Jason Merrill Feb. 25, 2014, 6:53 p.m. UTC 
The primary bug under discussion in 53808 has been fixed separately, but 
it also pointed out that once devirtualization resolves the delete to 
use the bar destructor, we ought to be able to inline that destructor. 
So if we're devirtualizing, always add a virtual defaulted dtor to the 
list of functions to be synthesized.

Tested x86_64-pc-linux-gnu, applying to trunk.

Comments

Andreas Schwab Feb. 27, 2014, 10:56 a.m. UTC | #1 
Jason Merrill <jason@redhat.com> writes:

> diff --git a/gcc/testsuite/g++.dg/opt/devirt4.C b/gcc/testsuite/g++.dg/opt/devirt4.C
> new file mode 100644
> index 0000000..26e8ee6
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/opt/devirt4.C
> @@ -0,0 +1,16 @@
> +// PR c++/53808
> +// Devirtualization + inlining should produce a non-virtual
> +// call to ~foo.
> +// { dg-options "-O -fdevirtualize" }
> +// { dg-final { scan-assembler "_ZN3fooD2Ev" } }
> +
> +struct foo {
> + virtual ~foo();
> +};
> +struct bar : public foo {
> + virtual void zed();
> +};
> +void f() {
> + foo *x(new bar);
> + delete x;
> +}

$ gcc/xg++ -Bgcc/ ../gcc/testsuite/g++.dg/opt/devirt4.C -nostdinc++ -Iia64-suse-linux/libstdc++-v3/include/ia64-suse-linux -Iia64-suse-linux/libstdc++-v3/include -Ilibstdc++-v3/libsupc++ -I../libstdc++-v3/include/backward -I../libstdc++-v3/testsuite/util -std=gnu++11 -O -fdevirtualize -ffat-lto-objects -S -o devirt4.s
$ cat devirt4.s 
        .file   "devirt4.C"
        .pred.safe_across_calls p1-p5,p16-p63
        .sdata
        .align 8
.LC0:
        data8   _ZTV3bar#+16
        .align 8
.LC1:
        data8   _ZTV3bar#+32
        .text
        .align 16
        .global _Z1fv#
        .type   _Z1fv#, @function
        .proc _Z1fv#
_Z1fv:
[.LFB0:]
        .prologue 12, 32
        .save ar.pfs, r33
        alloc r33 = ar.pfs, 0, 3, 1, 0
        .save rp, r32
        mov r32 = b0
        mov r34 = r1
        .body
        addl r35 = 8, r0
        ;;
        br.call.sptk.many b0 = _Znwm#
        mov r1 = r34
        ;;
        addl r14 = @gprel(.LC0), gp
        ;;
        ld8 r14 = [r14]
        ;;
        st8 [r8] = r14
        cmp.eq p6, p7 = 0, r8
        (p6) br.cond.dpnt .L1
        mov r35 = r8
        addl r14 = @gprel(.LC1), gp
        ;;
        ld8 r14 = [r14]
        ;;
        ld8 r15 = [r14], 8
        ;;
        mov b6 = r15
        ld8 r1 = [r14]
        br.call.sptk.many b0 = b6
        ;;
        mov r1 = r34
.L1:
        mov ar.pfs = r33
        mov b0 = r32
        br.ret.sptk.many b0
        ;;
        .endp _Z1fv#
        .ident  "GCC: (GNU) 4.9.0 20140227 (experimental) [trunk revision 208194]"

Andreas.
Jason Merrill Feb. 27, 2014, 1:49 p.m. UTC | #2 
Hmm, I wonder why we aren't devirtualizing that call on ia64.  Does it 
work with -O3?

Jason
Andreas Schwab Feb. 27, 2014, 1:53 p.m. UTC | #3 
Jason Merrill <jason@redhat.com> writes:

> Hmm, I wonder why we aren't devirtualizing that call on ia64.  Does it
> work with -O3?

That doesn't change anything fundamentally.

Andreas.
Richard Biener Feb. 27, 2014, 2:03 p.m. UTC | #4 
On Thu, Feb 27, 2014 at 2:53 PM, Andreas Schwab <schwab@suse.de> wrote:
> Jason Merrill <jason@redhat.com> writes:
>
>> Hmm, I wonder why we aren't devirtualizing that call on ia64.  Does it
>> work with -O3?
>
> That doesn't change anything fundamentally.

I think the vtable lookup sequence is different and nobody cared to adjust
the gimple matcher to also match the ia64 sequence.

Richard.

> Andreas.
>
> --
> Andreas Schwab, SUSE Labs, schwab@suse.de
> GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
> "And now for something completely different."
Jason Merrill Feb. 27, 2014, 2:51 p.m. UTC | #5 
On 02/27/2014 09:03 AM, Richard Biener wrote:
>> Jason Merrill <jason@redhat.com> writes:
>>> Hmm, I wonder why we aren't devirtualizing that call on ia64.
> I think the vtable lookup sequence is different and nobody cared to adjust
> the gimple matcher to also match the ia64 sequence.

Ah.  So xfail on ia64?

Jason
Richard Biener Feb. 27, 2014, 3 p.m. UTC | #6 
On Thu, Feb 27, 2014 at 3:51 PM, Jason Merrill <jason@redhat.com> wrote:
> On 02/27/2014 09:03 AM, Richard Biener wrote:
>>>
>>> Jason Merrill <jason@redhat.com> writes:
>>>>
>>>> Hmm, I wonder why we aren't devirtualizing that call on ia64.
>>
>> I think the vtable lookup sequence is different and nobody cared to adjust
>>
>> the gimple matcher to also match the ia64 sequence.
>
>
> Ah.  So xfail on ia64?

OTOH the g++dg/ipa/devirt-* testcases seem to run fine everywhere?

Richard.

> Jason
>
Martin Jambor Feb. 27, 2014, 4:38 p.m. UTC | #7 
On Thu, Feb 27, 2014 at 04:00:22PM +0100, Richard Biener wrote:
> On Thu, Feb 27, 2014 at 3:51 PM, Jason Merrill <jason@redhat.com> wrote:
> > On 02/27/2014 09:03 AM, Richard Biener wrote:
> >>>
> >>> Jason Merrill <jason@redhat.com> writes:
> >>>>
> >>>> Hmm, I wonder why we aren't devirtualizing that call on ia64.
> >>
> >> I think the vtable lookup sequence is different and nobody cared to adjust
> >>
> >> the gimple matcher to also match the ia64 sequence.
> >
> >
> > Ah.  So xfail on ia64?
> 
> OTOH the g++dg/ipa/devirt-* testcases seem to run fine everywhere?

This is probably the same issue as the one described in
http://gcc.gnu.org/ml/gcc/2012-08/msg00055.html and the subsequent
thread.  The problem however turned out to be slightly more difficult
and I was not interested enough in ia64 to care again.

Martin
Jan Hubicka Feb. 27, 2014, 7:18 p.m. UTC | #8 
> On Thu, Feb 27, 2014 at 2:53 PM, Andreas Schwab <schwab@suse.de> wrote:
> > Jason Merrill <jason@redhat.com> writes:
> >
> >> Hmm, I wonder why we aren't devirtualizing that call on ia64.  Does it
> >> work with -O3?
> >
> > That doesn't change anything fundamentally.
> 
> I think the vtable lookup sequence is different and nobody cared to adjust
> the gimple matcher to also match the ia64 sequence.

We sort of handle the function descriptors thorough the devirtualization code.
I will check why we fail here.

Honza
> 
> Richard.
> 
> > Andreas.
> >
> > --
> > Andreas Schwab, SUSE Labs, schwab@suse.de
> > GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
> > "And now for something completely different."
diff mbox

Patch

commit 9f9f907732429f413e490be0fa969b72153fdd88
Author: Jason Merrill <jason@redhat.com>
Date:   Fri Mar 22 20:24:49 2013 -0400

    	PR c++/53808
    	* class.c (clone_function_decl): Call note_vague_linkage_fn for
    	defaulted virtual dtor.

diff --git a/gcc/cp/class.c b/gcc/cp/class.c
index 97a1cc2..e861e4d 100644
--- a/gcc/cp/class.c
+++ b/gcc/cp/class.c
@@ -4582,6 +4582,10 @@  clone_function_decl (tree fn, int update_method_vec_p)
 	 destructor.  */
       if (DECL_VIRTUAL_P (fn))
 	{
+	  if (DECL_DEFAULTED_FN (fn) && flag_devirtualize)
+	    /* Make sure the destructor gets synthesized so that it can be
+	       inlined after devirtualization.  */
+	    note_vague_linkage_fn (fn);
 	  clone = build_clone (fn, deleting_dtor_identifier);
 	  if (update_method_vec_p)
 	    add_method (DECL_CONTEXT (clone), clone, NULL_TREE);
diff --git a/gcc/testsuite/g++.dg/opt/devirt4.C b/gcc/testsuite/g++.dg/opt/devirt4.C
new file mode 100644
index 0000000..26e8ee6
--- /dev/null
+++ b/gcc/testsuite/g++.dg/opt/devirt4.C
@@ -0,0 +1,16 @@ 
+// PR c++/53808
+// Devirtualization + inlining should produce a non-virtual
+// call to ~foo.
+// { dg-options "-O -fdevirtualize" }
+// { dg-final { scan-assembler "_ZN3fooD2Ev" } }
+
+struct foo {
+ virtual ~foo();
+};
+struct bar : public foo {
+ virtual void zed();
+};
+void f() {
+ foo *x(new bar);
+ delete x;
+}