Re: C++ PATCH for lto/53808 (devirtualization of defaulted virtual dtor)
> On Thu, Feb 27, 2014 at 2:53 PM, Andreas Schwab wrote: > > Jason Merrill 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, sch...@suse.de > > GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE 1748 E4D4 88E3 0EEA B9D7 > > "And now for something completely different."
Re: C++ PATCH for lto/53808 (devirtualization of defaulted virtual dtor)
On Thu, Feb 27, 2014 at 04:00:22PM +0100, Richard Biener wrote: > On Thu, Feb 27, 2014 at 3:51 PM, Jason Merrill wrote: > > On 02/27/2014 09:03 AM, Richard Biener wrote: > >>> > >>> Jason Merrill 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
Re: C++ PATCH for lto/53808 (devirtualization of defaulted virtual dtor)
On Thu, Feb 27, 2014 at 3:51 PM, Jason Merrill wrote: > On 02/27/2014 09:03 AM, Richard Biener wrote: >>> >>> Jason Merrill 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 >
Re: C++ PATCH for lto/53808 (devirtualization of defaulted virtual dtor)
On 02/27/2014 09:03 AM, Richard Biener wrote: Jason Merrill 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
Re: C++ PATCH for lto/53808 (devirtualization of defaulted virtual dtor)
On Thu, Feb 27, 2014 at 2:53 PM, Andreas Schwab wrote: > Jason Merrill 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, sch...@suse.de > GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE 1748 E4D4 88E3 0EEA B9D7 > "And now for something completely different."
Re: C++ PATCH for lto/53808 (devirtualization of defaulted virtual dtor)
Jason Merrill 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. -- Andreas Schwab, SUSE Labs, sch...@suse.de GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE 1748 E4D4 88E3 0EEA B9D7 "And now for something completely different."
Re: C++ PATCH for lto/53808 (devirtualization of defaulted virtual dtor)
Hmm, I wonder why we aren't devirtualizing that call on ia64. Does it work with -O3? Jason
Re: C++ PATCH for lto/53808 (devirtualization of defaulted virtual dtor)
Jason Merrill writes:
> diff --git a/gcc/testsuite/g++.dg/opt/devirt4.C
> b/gcc/testsuite/g++.dg/opt/devirt4.C
> new file mode 100644
> index 000..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.
--
Andreas Schwab, SUSE Labs, sch...@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE 1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."
Re: C++ PATCH for lto/53808 (devirtualization of defaulted virtual dtor)
> But this caused bug 60347: turns out that we shouldn't do this > unless the vtable (and thus the contents of the vtable) are used. The ipa-devirt type inheritance builder will use any vtable it finds in DECL_BINFO of types that it knows about. It starts with types of virtual methods and virtual tables in the symbol table (so we should be sure that TREE_USED is set for vtable that is associated with virtual method is set when virtual method itself is used). But it will drop in also all base types of those types and then it will take types found in OBJ_TYPE_REF & types from variables and type arguments. Are all those safe? I tried to describe this in http://hubicka.blogspot.ca/2014/02/devirtualization-in-c-part-3-building.html Honza
Re: C++ PATCH for lto/53808 (devirtualization of defaulted virtual dtor)
On 02/25/2014 01:53 PM, Jason Merrill wrote:
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.
But this caused bug 60347: turns out that we shouldn't do this unless
the vtable (and thus the contents of the vtable) are used.
Tested x86_64-pc-linux-gnu, applying to trunk.
commit 0e5db39431e8f66255a2566f22054878b18baf08
Author: Jason Merrill
Date: Wed Feb 26 15:40:42 2014 -0500
PR c++/60347
PR lto/53808
* class.c (clone_function_decl): Don't note_vague_linkage_fn.
* init.c (build_vtbl_address): Do it here.
diff --git a/gcc/cp/class.c b/gcc/cp/class.c
index f61dc9d..b46391b 100644
--- a/gcc/cp/class.c
+++ b/gcc/cp/class.c
@@ -4584,10 +4584,6 @@ 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/cp/init.c b/gcc/cp/init.c
index 194a797..3ae2b5c 100644
--- a/gcc/cp/init.c
+++ b/gcc/cp/init.c
@@ -1123,7 +1123,13 @@ build_vtbl_address (tree binfo)
/* Figure out what vtable BINFO's vtable is based on, and mark it as
used. */
vtbl = get_vtbl_decl_for_binfo (binfo_for);
- TREE_USED (vtbl) = 1;
+ if (tree dtor = CLASSTYPE_DESTRUCTORS (DECL_CONTEXT (vtbl)))
+if (!TREE_USED (vtbl) && DECL_VIRTUAL_P (dtor) && DECL_DEFAULTED_FN (dtor))
+ /* Make sure the destructor gets synthesized so that it can be
+ inlined after devirtualization even if the vtable is never
+ emitted. */
+ note_vague_linkage_fn (dtor);
+ TREE_USED (vtbl) = true;
/* Now compute the address to use when initializing the vptr. */
vtbl = unshare_expr (BINFO_VTABLE (binfo_for));
diff --git a/gcc/testsuite/g++.dg/template/dtor9.C b/gcc/testsuite/g++.dg/template/dtor9.C
new file mode 100644
index 000..fd71389
--- /dev/null
+++ b/gcc/testsuite/g++.dg/template/dtor9.C
@@ -0,0 +1,12 @@
+// PR c++/60347
+
+struct A;
+
+template
+struct B
+{
+ T* p;
+ virtual ~B() { p->~T(); }
+};
+
+struct C: B { };
