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? 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