Jakub Jelinek <ja...@redhat.com> wrote:
>On Tue, Dec 31, 2013 at 11:30:12AM +0100, Richard Biener wrote:
>> Meanwhile your patch is ok.
>
>As discussed in the PR, the patch wasn't sufficient, __cxa_pure_virtual
>can appear in the vtable and it has pretty much the same properties
>as __builtin_unreachable (void return value, no arguments, noreturn,
>DCE or cfg cleanup being able to remove anything after it because of
>noreturn).
>
>Additionally, the patch attempts to punt on invalid type changes
>(ODR violations?, apparently none appear during bootstrap/regtest as
>verified by additional logging added there) or inplace changes of
>gimple_call_flags that would require some cleanups caller isn't
>expected to
>do (again, doesn't happen during bootstrap/regtest).
>
>Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
I'd rather do nothing in the !inplace case, it's rare enough to not care.
Also there is nothing wrong with wrong types et al - we have a perfect
mechanism for dealing with this. Change the fndecl but not the fntype.
I'd like to see the code simplified with keeping the above in mind.
Thanks,
Richard.
>2014-01-03 Jakub Jelinek <ja...@redhat.com>
>
> PR tree-optimization/59622
> * gimple-fold.c: Include calls.h.
> (gimple_fold_call): Fix a typo in message. Handle
> __cxa_pure_virtual similarly to __builtin_unreachable. For
> inplace punt if gimple_call_flags would change. Verify
> that lhs and argument types are compatible.
>
> * g++.dg/opt/pr59622-2.C: New test.
>
>--- gcc/gimple-fold.c.jj 2013-12-31 12:56:59.000000000 +0100
>+++ gcc/gimple-fold.c 2014-01-02 18:33:51.207921734 +0100
>@@ -51,6 +51,7 @@ along with GCC; see the file COPYING3.
> #include "gimple-pretty-print.h"
> #include "tree-ssa-address.h"
> #include "langhooks.h"
>+#include "calls.h"
>
> /* Return true when DECL can be referenced from current unit.
>FROM_DECL (if non-null) specify constructor of variable DECL was taken
>from.
>@@ -1167,7 +1168,7 @@ gimple_fold_call (gimple_stmt_iterator *
> (OBJ_TYPE_REF_EXPR (callee)))))
> {
> fprintf (dump_file,
>- "Type inheritnace inconsistent devirtualization of ");
>+ "Type inheritance inconsistent devirtualization of ");
> print_gimple_stmt (dump_file, stmt, 0, TDF_SLIM);
> fprintf (dump_file, " to ");
> print_generic_expr (dump_file, callee, TDF_SLIM);
>@@ -1184,18 +1185,74 @@ gimple_fold_call (gimple_stmt_iterator *
> = possible_polymorphic_call_targets (callee, &final);
> if (final && targets.length () <= 1)
> {
>+ tree fndecl;
>+ int call_flags = gimple_call_flags (stmt), new_flags;
> if (targets.length () == 1)
>+ fndecl = targets[0]->decl;
>+ else
>+ fndecl = builtin_decl_implicit (BUILT_IN_UNREACHABLE);
>+ new_flags = flags_from_decl_or_type (fndecl);
>+ if (inplace)
> {
>- gimple_call_set_fndecl (stmt, targets[0]->decl);
>- changed = true;
>+ /* For inplace, avoid changes of call flags that
>+ might require cfg cleanups, EH cleanups, removal
>+ of vdef/vuses etc. */
>+ if ((call_flags ^ new_flags)
>+ & (ECF_NORETURN | ECF_NOTHROW | ECF_NOVOPS))
>+ final = false;
>+ else if ((call_flags & ECF_NOVOPS) == 0)
>+ {
>+ if ((call_flags ^ new_flags) & ECF_CONST)
>+ final = false;
>+ else if (((call_flags & (ECF_PURE | ECF_CONST
>+ | ECF_NORETURN)) == 0)
>+ ^ ((new_flags & (ECF_PURE | ECF_CONST
>+ | ECF_NORETURN)) == 0))
>+ final = false;
>+ }
> }
>- else if (!inplace)
>+ if (final)
> {
>- tree fndecl = builtin_decl_implicit (BUILT_IN_UNREACHABLE);
>- gimple new_stmt = gimple_build_call (fndecl, 0);
>- gimple_set_location (new_stmt, gimple_location (stmt));
>- gsi_insert_before (gsi, new_stmt, GSI_SAME_STMT);
>- return true;
>+ tree fntype = gimple_call_fntype (stmt);
>+ tree dtype = TREE_TYPE (fndecl);
>+ if (gimple_call_lhs (stmt)
>+ && !useless_type_conversion_p (TREE_TYPE (fntype),
>+ TREE_TYPE (dtype)))
>+ final = false;
>+ else
>+ {
>+ tree t1, t2;
>+ for (t1 = TYPE_ARG_TYPES (fntype),
>+ t2 = TYPE_ARG_TYPES (dtype);
>+ t1 && t2;
>+ t1 = TREE_CHAIN (t1), t2 = TREE_CHAIN (t2))
>+ if (!useless_type_conversion_p (TREE_VALUE (t2),
>+ TREE_VALUE (t1)))
>+ break;
>+ if (t1 || t2)
>+ final = false;
>+ }
>+
>+ /* If fndecl (like __builtin_unreachable or
>+ __cxa_pure_virtual) takes no arguments, doesn't have
>+ return value and is noreturn, just add the call before
>+ stmt and DCE will do it's job later on. */
>+ if (!final
>+ && !inplace
>+ && (new_flags & ECF_NORETURN)
>+ && VOID_TYPE_P (TREE_TYPE (dtype))
>+ && TYPE_ARG_TYPES (dtype) == void_list_node)
>+ {
>+ gimple new_stmt = gimple_build_call (fndecl, 0);
>+ gimple_set_location (new_stmt, gimple_location (stmt));
>+ gsi_insert_before (gsi, new_stmt, GSI_SAME_STMT);
>+ return true;
>+ }
>+ }
>+ if (final)
>+ {
>+ gimple_call_set_fndecl (stmt, fndecl);
>+ changed = true;
> }
> }
> }
>--- gcc/testsuite/g++.dg/opt/pr59622-2.C.jj 2014-01-02
>18:24:30.422832814 +0100
>+++ gcc/testsuite/g++.dg/opt/pr59622-2.C 2014-01-02 18:33:29.052037954
>+0100
>@@ -0,0 +1,21 @@
>+// PR tree-optimization/59622
>+// { dg-do compile }
>+// { dg-options "-O2" }
>+
>+namespace
>+{
>+ struct A
>+ {
>+ A () {}
>+ virtual A *bar (int) = 0;
>+ A *baz (int x) { return bar (x); }
>+ };
>+}
>+
>+A *a;
>+
>+void
>+foo ()
>+{
>+ a->baz (0);
>+}
>
>
> Jakub
