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 PR bootstrap/58509


Hi,

this fixes the ICE during the build of the Ada runtime on the SPARC, a fallout 
of the recent inliner changes:
  http://gcc.gnu.org/ml/gcc-patches/2013-09/msg01033.html

The ICE is triggered because the ldd peephole merges an MEM with MEM_NOTRAP_P 
and a contiguous MEM without MEM_NOTRAP_P, keeping the MEM_NOTRAP_P flag on 
the result.  As a consequence, an EH edge is eliminated and a BB is orphaned.

I think this shows that my above inliner patch was too gross: when you have 
successive inlining, you can quickly end up with a mess of trapping and non-
trapping memory accesses for the same object.  So the attached seriously 
refines it, restricting it to parameters with reference type and leaning 
towards being less conservative.  Again, this should only affect Ada.

Tested on x86_64-suse-linux, OK for the mainline?


2013-09-27  Eric Botcazou  <ebotcazou@adacore.com>

	PR bootstrap/58509
	* ipa-prop.h (get_ancestor_addr_info): Declare.
	* ipa-prop.c (get_ancestor_addr_info): Make public.
	* tree-inline.c (is_parm): Rename into...
	(is_ref_parm): ...this.
	(is_based_on_ref_parm): New predicate.
	(remap_gimple_op_r): Do not propagate TREE_THIS_NOTRAP on MEM_REF if
	a parameter with reference type has been remapped and the result is
	not based on another parameter with reference type.
	(copy_tree_body_r): Likewise on INDIRECT_REF and MEM_REF.


2013-09-27  Eric Botcazou  <ebotcazou@adacore.com>

	* gnat.dg/specs/opt1.ads: New test.


-- 
Eric Botcazou
Index: tree-inline.c
===================================================================
--- tree-inline.c	(revision 202912)
+++ tree-inline.c	(working copy)
@@ -751,10 +751,11 @@ copy_gimple_bind (gimple stmt, copy_body
   return new_bind;
 }
 
-/* Return true if DECL is a parameter or a SSA_NAME for a parameter.  */
+/* Return true if DECL is a parameter with reference type or a SSA_NAME
+  for a parameter with reference type.  */
 
 static bool
-is_parm (tree decl)
+is_ref_parm (tree decl)
 {
   if (TREE_CODE (decl) == SSA_NAME)
     {
@@ -763,7 +764,40 @@ is_parm (tree decl)
 	return false;
     }
 
-  return (TREE_CODE (decl) == PARM_DECL);
+  return (TREE_CODE (decl) == PARM_DECL
+	  && TREE_CODE (TREE_TYPE (decl)) == REFERENCE_TYPE);
+}
+
+/* Return true if DECL is based on a parameter with reference type or a
+   SSA_NAME for a parameter with with reference type.  */
+
+static bool
+is_based_on_ref_parm (tree decl)
+{
+  HOST_WIDE_INT offset;
+  tree obj, expr;
+  gimple def_stmt;
+
+  /* First the easy case.  */
+  if (is_ref_parm (decl))
+    return true;
+
+  /* Then look for an SSA name whose defining statement is of the form:
+
+      D.1718_7 = &parm_2(D)->f1;
+
+     where parm_2 is a parameter with reference type.  */
+  if (TREE_CODE (decl) != SSA_NAME)
+    return false;
+  def_stmt = SSA_NAME_DEF_STMT (decl);
+  if (!def_stmt)
+    return false;
+
+  expr = get_ancestor_addr_info (def_stmt, &obj, &offset);
+  if (!expr)
+    return false;
+
+  return is_ref_parm (TREE_OPERAND (expr, 0));
 }
 
 /* Remap the GIMPLE operand pointed to by *TP.  DATA is really a
@@ -865,12 +899,13 @@ remap_gimple_op_r (tree *tp, int *walk_s
 	  TREE_THIS_VOLATILE (*tp) = TREE_THIS_VOLATILE (old);
 	  TREE_SIDE_EFFECTS (*tp) = TREE_SIDE_EFFECTS (old);
 	  TREE_NO_WARNING (*tp) = TREE_NO_WARNING (old);
-	  /* We cannot propagate the TREE_THIS_NOTRAP flag if we have
-	     remapped a parameter as the property might be valid only
-	     for the parameter itself.  */
+	  /* We cannot always propagate the TREE_THIS_NOTRAP flag if we have
+	     remapped a parameter with reference type as the property may be
+	     valid only for the parameter.  */
 	  if (TREE_THIS_NOTRAP (old)
-	      && (!is_parm (TREE_OPERAND (old, 0))
-		  || (!id->transform_parameter && is_parm (ptr))))
+	      && (!is_ref_parm (TREE_OPERAND (old, 0))
+		  || !id->transform_parameter
+		  || is_based_on_ref_parm (ptr)))
 	    TREE_THIS_NOTRAP (*tp) = 1;
 	  *walk_subtrees = 0;
 	  return NULL;
@@ -1092,12 +1127,13 @@ copy_tree_body_r (tree *tp, int *walk_su
 		      TREE_THIS_VOLATILE (*tp) = TREE_THIS_VOLATILE (old);
 		      TREE_SIDE_EFFECTS (*tp) = TREE_SIDE_EFFECTS (old);
 		      TREE_READONLY (*tp) = TREE_READONLY (old);
-		      /* We cannot propagate the TREE_THIS_NOTRAP flag if we
-			 have remapped a parameter as the property might be
-			 valid only for the parameter itself.  */
+		      /* We cannot always propagate the TREE_THIS_NOTRAP flag
+			 if we have remapped a parameter with reference type as
+			 the property may be valid only for the parameter.  */
 		      if (TREE_THIS_NOTRAP (old)
-			  && (!is_parm (TREE_OPERAND (old, 0))
-			      || (!id->transform_parameter && is_parm (ptr))))
+			  && (!is_ref_parm (TREE_OPERAND (old, 0))
+			      || !id->transform_parameter
+			      || is_based_on_ref_parm (ptr)))
 		        TREE_THIS_NOTRAP (*tp) = 1;
 		    }
 		}
@@ -1118,12 +1154,13 @@ copy_tree_body_r (tree *tp, int *walk_su
 	  TREE_THIS_VOLATILE (*tp) = TREE_THIS_VOLATILE (old);
 	  TREE_SIDE_EFFECTS (*tp) = TREE_SIDE_EFFECTS (old);
 	  TREE_NO_WARNING (*tp) = TREE_NO_WARNING (old);
-	  /* We cannot propagate the TREE_THIS_NOTRAP flag if we have
-	     remapped a parameter as the property might be valid only
-	     for the parameter itself.  */
+	  /* We cannot always propagate the TREE_THIS_NOTRAP flag if we have
+	     remapped a parameter with reference type as the property may be
+	     valid only for the parameter.  */
 	  if (TREE_THIS_NOTRAP (old)
-	      && (!is_parm (TREE_OPERAND (old, 0))
-		  || (!id->transform_parameter && is_parm (ptr))))
+	      && (!is_ref_parm (TREE_OPERAND (old, 0))
+		  || !id->transform_parameter
+		  || is_based_on_ref_parm (ptr)))
 	    TREE_THIS_NOTRAP (*tp) = 1;
 	  *walk_subtrees = 0;
 	  return NULL;
Index: ipa-prop.h
===================================================================
--- ipa-prop.h	(revision 202912)
+++ ipa-prop.h	(working copy)
@@ -693,6 +693,7 @@ tree ipa_value_from_jfunc (struct ipa_no
 unsigned int ipcp_transform_function (struct cgraph_node *node);
 void ipa_dump_param (FILE *, struct ipa_node_params *info, int i);
 
+tree get_ancestor_addr_info (gimple, tree *, HOST_WIDE_INT *);
 
 /* From tree-sra.c:  */
 tree build_ref_for_offset (location_t, tree, HOST_WIDE_INT, tree,
Index: ipa-prop.c
===================================================================
--- ipa-prop.c	(revision 202912)
+++ ipa-prop.c	(working copy)
@@ -1078,7 +1078,7 @@ compute_complex_assign_jump_func (struct
    handled components and the MEM_REF itself is stored into *OFFSET.  The whole
    RHS stripped off the ADDR_EXPR is stored into *OBJ_P.  */
 
-static tree
+tree
 get_ancestor_addr_info (gimple assign, tree *obj_p, HOST_WIDE_INT *offset)
 {
   HOST_WIDE_INT size, max_size;
-- { dg-do compile }
-- { dg-options "-O2" }

with Ada.Strings.Unbounded; use Ada.Strings.Unbounded;

package Opt1 is

   type Ptr is access all Integer;

   type R1 is record
      I1 : Integer;
      I2 : Integer := 0;
      I3 : Integer;
   end record;

   type R2 is record
      P  : Ptr;
      F1 : R1;
   end record;

   type R3 is record
      S  : Unbounded_String;
      F1 : R2;
      I  : Integer := 0;
      F2 : R2;
   end record;

end Opt1;

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