This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[PATCH] Fix devirtualization ICE (PR tree-optimization/59622, take 2)


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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]