This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
[PATCH] Fix devirtualization ICE (PR tree-optimization/59622, take 2)
- From: Jakub Jelinek <jakub at redhat dot com>
- To: Richard Biener <rguenther at suse dot de>
- Cc: Jan Hubicka <hubicka at ucw dot cz>, gcc-patches at gcc dot gnu dot org
- Date: Fri, 3 Jan 2014 09:31:21 +0100
- Subject: [PATCH] Fix devirtualization ICE (PR tree-optimization/59622, take 2)
- Authentication-results: sourceware.org; auth=none
- References: <20131230164723 dot GD892 at tucnak dot redhat dot com> <02e66dc6-bc24-4ffc-ae2f-affd1ec287e5 at email dot android dot com> <20131230211416 dot GA27228 at kam dot mff dot cuni dot cz> <20131230222412 dot GF892 at tucnak dot redhat dot com> <47adfd09-7411-4aa5-9782-e888833c6be9 at email dot android dot com> <20131231093208 dot GG892 at tucnak dot redhat dot com> <3246dbc4-3dfa-47d4-b2ff-d36d2f848c1c at email dot android dot com>
- Reply-to: Jakub Jelinek <jakub at redhat dot com>
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 <jakub@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