> On 02/28/2014 03:56 PM, Jan Hubicka wrote:
> >I think we can safely test here DECL_ARTIFICIAL && (DECL_EXTERNAL ||
> >DECL_COMDAT). If the dtor is going to be output anyway, we are safe to use
> >it.
>
> We already skipped DECL_EXTERNAL decls, and artificial members are
> always DECL_COMDAT, but I'll add the COMDAT check.
>
> >Are those programs valid by C++ standard? (I believe it is not valid to
> >include
> >stuff whose implementation you do not link with.).
>
> Symbol visibility is outside the scope of the standard.
>
> >If we just want to avoid
> >breaking python and libreoffice (I fixed libreoffice part however), we may
> >just
> >go with the ipa-devirt change as you propose (with external&comdat check).
> >
> >If this is an correctness issue, I think we want to be safe that other
> >optimizations
> >won't do the same. In that case your check seems misplaced.
> >
> >If DECL_ARTIFICIAL destructors are not safe to inline, I would add it into
> >function_attribute_inlinable_p. If the dtor is not safe to refer, then I
> >would
> >add it into can_refer_decl_in_current_unit_p
>
> >Both such changes would however inhibit quite some optimization, since
> >artificial destructors are quite common case, right? Or is there some reason
> >why
> >only speculative devirtualization count possibly work out reference to these?
>
> Normally, it's fine to inline destructors, and refer to them. The
> problem comes when we turn what had been a virtual call (which goes
> through the vtable that is hidden in the DSO) into a direct call to
> a hidden function. We don't do that for user-defined virtual
> functions because the user controls whether or not they are defined
> in the header, and we don't devirtualize if no definition is
> available, but implicitly-declared functions are different because
> the user has no way to prevent the definition from being available.
>
> This also isn't a problem for cprop devirtualization, because in
> that situation we must have already referred to the vtable.
Is this always the case? For normal virtual method you can have something
like:
foo(class bla a)
{
a->bar();
}
Here we will devirutalize bar into bla's method without ever seeing the vtable.
I am not sure if I can construct a testcase for dtor w/o referencing the
vtable, since generally those are cases where dtor is generated autmatically.
But it is a bit fragile assumption to be made.
Jan
>
> Jason
>
> commit 2a05a09c268ce3abb373aa86cf731d20aac8dd7a
> Author: Jason Merrill <[email protected]>
> Date: Fri Feb 28 14:03:19 2014 -0500
>
> PR c++/58678
> * ipa-devirt.c (ipa_devirt): Don't choose an implicitly-declared
> function.
>
> diff --git a/gcc/ipa-devirt.c b/gcc/ipa-devirt.c
> index 21649cb..2f84f17 100644
> --- a/gcc/ipa-devirt.c
> +++ b/gcc/ipa-devirt.c
> @@ -1710,7 +1710,7 @@ ipa_devirt (void)
>
> int npolymorphic = 0, nspeculated = 0, nconverted = 0, ncold = 0;
> int nmultiple = 0, noverwritable = 0, ndevirtualized = 0, nnotdefined = 0;
> - int nwrong = 0, nok = 0, nexternal = 0;;
> + int nwrong = 0, nok = 0, nexternal = 0, nartificial = 0;
>
> FOR_EACH_DEFINED_FUNCTION (n)
> {
> @@ -1820,6 +1820,17 @@ ipa_devirt (void)
> nexternal++;
> continue;
> }
> + /* Don't use an implicitly-declared destructor (c++/58678). */
> + struct cgraph_node *non_thunk_target
> + = cgraph_function_node (likely_target);
> + if (DECL_ARTIFICIAL (non_thunk_target->decl)
> + && DECL_COMDAT (non_thunk_target->decl))
> + {
> + if (dump_file)
> + fprintf (dump_file, "Target is artificial\n\n");
> + nartificial++;
> + continue;
> + }
> if (cgraph_function_body_availability (likely_target)
> <= AVAIL_OVERWRITABLE
> && symtab_can_be_discarded (likely_target))
> @@ -1862,10 +1873,10 @@ ipa_devirt (void)
> " %i speculatively devirtualized, %i cold\n"
> "%i have multiple targets, %i overwritable,"
> " %i already speculated (%i agree, %i disagree),"
> - " %i external, %i not defined\n",
> + " %i external, %i not defined, %i artificial\n",
> npolymorphic, ndevirtualized, nconverted, ncold,
> nmultiple, noverwritable, nspeculated, nok, nwrong,
> - nexternal, nnotdefined);
> + nexternal, nnotdefined, nartificial);
> return ndevirtualized ? TODO_remove_functions : 0;
> }
>
> diff --git a/gcc/testsuite/g++.dg/ipa/devirt-28.C
> b/gcc/testsuite/g++.dg/ipa/devirt-28.C
> new file mode 100644
> index 0000000..e18b818
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/ipa/devirt-28.C
> @@ -0,0 +1,17 @@
> +// PR c++/58678
> +// { dg-options "-O3 -fdump-ipa-devirt" }
> +
> +struct A {
> + virtual ~A();
> +};
> +struct B : A {
> + virtual int m_fn1();
> +};
> +void fn1(B* b) {
> + delete b;
> +}
> +
> +// { dg-final { scan-assembler-not "_ZN1AD2Ev" } }
> +// { dg-final { scan-assembler-not "_ZN1BD0Ev" } }
> +// { dg-final { scan-ipa-dump "Target is artificial" "devirt" } }
> +// { dg-final { cleanup-ipa-dump "devirt" } }
