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]

Fix tree-optimization/62091


Hi,
this should be hopefully last fix to tree-optimization/62091.  The testcase triggers
another unnecesary paranoia in get_dynamic_type: if location is of type A to start with
and then it is re-initialized to type A, it should not count as dynamic type change.

While looking into it, I added some debug info that I think is useful enough to be kept
and I also noticed that we do not handle multiple inheritance very well. Fixed thus.

Bootstrapped/regtested x86_64-linux, will commit it after bit of additional testing.
With this change I can now build libreoffice, so I think I can drop the old ipa-prop
intraprocedural code and remove the sanity check.

Honza

	PR tree-optimization/62091
	* g++.dg/ipa/devirt-37.C: Update template.
	* g++.dg/ipa/devirt-40.C: New testcase.
	* ipa-devirt.c (ipa_polymorphic_call_context::restrict_to_inner_type):
	handle correctly arrays.
	(extr_type_from_vtbl_ptr_store): Add debug output; handle multiple
	inheritance binfos.
	(record_known_type): Walk into inner type.
	(ipa_polymorphic_call_context::get_dynamic_type): Likewise; strenghten
	condition on no type changes.
Index: testsuite/g++.dg/ipa/devirt-37.C
===================================================================
--- testsuite/g++.dg/ipa/devirt-37.C	(revision 214255)
+++ testsuite/g++.dg/ipa/devirt-37.C	(working copy)
@@ -30,7 +30,7 @@ t()
 /* After inlining the call within constructor needs to be checked to not go into a basetype.
    We should see the vtbl store and we should notice extcall as possibly clobbering the
    type but ignore it because b is in static storage.  */
-/* { dg-final { scan-tree-dump "Determined dynamic type."  "fre2"  } } */
+/* { dg-final { scan-tree-dump "No dynamic type change found."  "fre2"  } } */
 /* { dg-final { scan-tree-dump "Checking vtbl store:"  "fre2"  } } */
 /* { dg-final { scan-tree-dump "Function call may change dynamic type:extcall"  "fre2"  } } */
 /* { dg-final { scan-tree-dump "converting indirect call to function virtual void"  "fre2"  } } */
Index: testsuite/g++.dg/ipa/devirt-40.C
===================================================================
--- testsuite/g++.dg/ipa/devirt-40.C	(revision 0)
+++ testsuite/g++.dg/ipa/devirt-40.C	(revision 0)
@@ -0,0 +1,23 @@
+/* { dg-options "-O2 -fdump-tree-fre2-details"  } */
+typedef enum
+{
+} UErrorCode;
+class UnicodeString
+{
+public:
+  UnicodeString ();
+  virtual ~UnicodeString ();
+};
+class A
+{
+  UnicodeString &m_fn1 (UnicodeString &, int &p2, UErrorCode &) const;
+};
+UnicodeString::UnicodeString () {}
+UnicodeString &
+A::m_fn1 (UnicodeString &, int &p2, UErrorCode &) const
+{
+  UnicodeString a[2];
+}
+
+/* { dg-final { scan-tree-dump "converting indirect call to function virtual UnicodeString" "fre2"  } } */
+/* { dg-final { cleanup-tree-dump "fre2" } } */
Index: ipa-devirt.c
===================================================================
--- ipa-devirt.c	(revision 214223)
+++ ipa-devirt.c	(working copy)
@@ -2015,8 +2015,10 @@ ipa_polymorphic_call_context::restrict_t
 	  tree subtype = TYPE_MAIN_VARIANT (TREE_TYPE (type));
 
 	  /* Give up if we don't know array size.  */
-	  if (!tree_fits_shwi_p (TYPE_SIZE (subtype))
-	      || !tree_to_shwi (TYPE_SIZE (subtype)) <= 0)
+	  if (!TYPE_SIZE (subtype)
+	      || !tree_fits_shwi_p (TYPE_SIZE (subtype))
+	      || tree_to_shwi (TYPE_SIZE (subtype)) <= 0
+	      || !contains_polymorphic_type_p (subtype))
 	    goto give_up;
 	  cur_offset = cur_offset % tree_to_shwi (TYPE_SIZE (subtype));
 	  type = subtype;
@@ -2630,7 +2632,7 @@ extr_type_from_vtbl_ptr_store (gimple st
 			       HOST_WIDE_INT *type_offset)
 {
   HOST_WIDE_INT offset, size, max_size;
-  tree lhs, rhs, base, binfo;
+  tree lhs, rhs, base;
 
   if (!gimple_assign_single_p (stmt))
     return NULL_TREE;
@@ -2639,7 +2641,11 @@ extr_type_from_vtbl_ptr_store (gimple st
   rhs = gimple_assign_rhs1 (stmt);
   if (TREE_CODE (lhs) != COMPONENT_REF
       || !DECL_VIRTUAL_P (TREE_OPERAND (lhs, 1)))
-    return NULL_TREE;
+     {
+	if (dump_file)
+	  fprintf (dump_file, "  LHS is not virtual table.\n");
+	return NULL_TREE;
+     }
 
   if (tci->vtbl_ptr_ref && operand_equal_p (lhs, tci->vtbl_ptr_ref, 0))
     ;
@@ -2649,33 +2655,80 @@ extr_type_from_vtbl_ptr_store (gimple st
       if (offset != tci->offset
 	  || size != POINTER_SIZE
 	  || max_size != POINTER_SIZE)
-	return NULL_TREE;
+	{
+	  if (dump_file)
+	    fprintf (dump_file, "    wrong offset %i!=%i or size %i\n",
+		     (int)offset, (int)tci->offset, (int)size);
+	  return NULL_TREE;
+	}
       if (DECL_P (tci->instance))
 	{
 	  if (base != tci->instance)
-	    return NULL_TREE;
+	    {
+	      if (dump_file)
+		{
+		  fprintf (dump_file, "    base:");
+		  print_generic_expr (dump_file, base, TDF_SLIM);
+		  fprintf (dump_file, " does not match instance:");
+		  print_generic_expr (dump_file, tci->instance, TDF_SLIM);
+		  fprintf (dump_file, "\n");
+		}
+	      return NULL_TREE;
+	    }
 	}
       else if (TREE_CODE (base) == MEM_REF)
 	{
 	  if (!operand_equal_p (tci->instance, TREE_OPERAND (base, 0), 0)
 	      || !integer_zerop (TREE_OPERAND (base, 1)))
-	    return NULL_TREE;
+	    {
+	      if (dump_file)
+		{
+		  fprintf (dump_file, "    base mem ref:");
+		  print_generic_expr (dump_file, base, TDF_SLIM);
+		  fprintf (dump_file, " has nonzero offset or does not match instance:");
+		  print_generic_expr (dump_file, tci->instance, TDF_SLIM);
+		  fprintf (dump_file, "\n");
+		}
+	      return NULL_TREE;
+	    }
 	}
       else if (!operand_equal_p (tci->instance, base, 0)
 	       || tci->offset)
-	return NULL_TREE;
+	{
+	  if (dump_file)
+	    {
+	      fprintf (dump_file, "    base:");
+	      print_generic_expr (dump_file, base, TDF_SLIM);
+	      fprintf (dump_file, " does not match instance:");
+	      print_generic_expr (dump_file, tci->instance, TDF_SLIM);
+	      fprintf (dump_file, " with offset %i\n", (int)tci->offset);
+	    }
+	  return NULL_TREE;
+	}
     }
 
-  binfo = vtable_pointer_value_to_binfo (rhs);
+  tree vtable;
+  unsigned HOST_WIDE_INT offset2;
 
+  if (!vtable_pointer_value_to_vtable (rhs, &vtable, &offset2))
+    {
+      if (dump_file)
+	fprintf (dump_file, "    Failed to lookup binfo\n");
+      return NULL;
+    }
+
+  tree binfo = subbinfo_with_vtable_at_offset (TYPE_BINFO (DECL_CONTEXT (vtable)),
+					       offset2, vtable);
   if (!binfo)
-    return NULL;
+    {
+      if (dump_file)
+	fprintf (dump_file, "    Construction vtable used\n");
+      /* FIXME: We should suport construction contextes.  */
+      return NULL;
+    }
+ 
   *type_offset = tree_to_shwi (BINFO_OFFSET (binfo)) * BITS_PER_UNIT;
-  if (TYPE_BINFO (BINFO_TYPE (binfo)) == binfo)
-    return BINFO_TYPE (binfo);
-
-  /* TODO: Figure out the type containing BINFO.  */
-  return NULL;
+  return DECL_CONTEXT (vtable);
 }
 
 /* Record dynamic type change of TCI to TYPE.  */
@@ -2694,11 +2747,43 @@ record_known_type (struct type_change_in
      else
        fprintf (dump_file, "  Recording unknown type\n");
     }
+
+  /* If we found a constructor of type that is not polymorphic or
+     that may contain the type in question as a field (not as base),
+     restrict to the inner class first to make type matching bellow
+     happier.  */
+  if (type
+      && (offset
+          || (TREE_CODE (type) != RECORD_TYPE
+	      || !polymorphic_type_binfo_p (TYPE_BINFO (type)))))
+    {
+      ipa_polymorphic_call_context context;
+
+      context.offset = offset;
+      context.outer_type = type;
+      context.maybe_in_construction = false;
+      context.maybe_derived_type = false;
+      /* If we failed to find the inner type, we know that the call
+	 would be undefined for type produced here.  */
+      if (!context.restrict_to_inner_class (tci->otr_type))
+	{
+	  if (dump_file)
+	    fprintf (dump_file, "  Ignoring; does not contain otr_type\n");
+	  return;
+	}
+      /* Watch for case we reached an POD type and anticipate placement
+	 new.  */
+      if (!context.maybe_derived_type)
+	{
+          type = context.outer_type;
+          offset = context.offset;
+	}
+    }
   if (tci->type_maybe_changed
-      && (type != tci->known_current_type
+      && (!types_same_for_odr (type, tci->known_current_type)
 	  || offset != tci->known_current_offset))
     tci->multiple_types_encountered = true;
-  tci->known_current_type = type;
+  tci->known_current_type = TYPE_MAIN_VARIANT (type);
   tci->known_current_offset = offset;
   tci->type_maybe_changed = true;
 }
@@ -2846,6 +2931,20 @@ ipa_polymorphic_call_context::get_dynami
   bool function_entry_reached = false;
   tree instance_ref = NULL;
   gimple stmt = call;
+  /* Remember OFFSET before it is modified by restrict_to_inner_class.
+     This is because we do not update INSTANCE when walking inwards.  */
+  HOST_WIDE_INT instance_offset = offset;
+
+  otr_type = TYPE_MAIN_VARIANT (otr_type);
+
+  /* Walk into inner type. This may clear maybe_derived_type and save us
+     from useless work.  It also makes later comparsions with static type
+     easier.  */
+  if (outer_type)
+    {
+      if (!restrict_to_inner_class (otr_type))
+        return false;
+    }
 
   if (!maybe_in_construction && !maybe_derived_type)
     return false;
@@ -2900,11 +2999,11 @@ ipa_polymorphic_call_context::get_dynami
 		     or from INSTANCE with offset OFFSET.  */
 		  if (base_ref
 		      && ((TREE_CODE (base_ref) == MEM_REF
-		           && ((offset2 == offset
+		           && ((offset2 == instance_offset
 		                && TREE_OPERAND (base_ref, 0) == instance)
 			       || (!offset2 && TREE_OPERAND (base_ref, 0) == otr_object)))
 			  || (DECL_P (instance) && base_ref == instance
-			      && offset2 == offset)))
+			      && offset2 == instance_offset)))
 		    {
 		      stmt = SSA_NAME_DEF_STMT (ref);
 		      instance_ref = ref_exp;
@@ -3010,7 +3109,13 @@ ipa_polymorphic_call_context::get_dynami
      only if there was dyanmic type store that may affect given variable
      (seen_unanalyzed_store)  */
 
-  if (!tci.type_maybe_changed)
+  if (!tci.type_maybe_changed
+      || (outer_type
+	  && !tci.seen_unanalyzed_store
+	  && !tci.multiple_types_encountered
+	  && offset == tci.offset
+	  && types_same_for_odr (tci.known_current_type,
+				 outer_type)))
     {
       if (!outer_type || tci.seen_unanalyzed_store)
 	return false;
@@ -3025,16 +3130,9 @@ ipa_polymorphic_call_context::get_dynami
       && !function_entry_reached
       && !tci.multiple_types_encountered)
     {
-      if (!tci.speculative
-	  /* Again in instances located in static storage we are interested only
-	     in constructor stores.  */
-	  || (outer_type
-	      && !tci.seen_unanalyzed_store
-	      && offset == tci.offset
-	      && types_same_for_odr (tci.known_current_type,
-				     outer_type)))
+      if (!tci.speculative)
 	{
-	  outer_type = tci.known_current_type;
+	  outer_type = TYPE_MAIN_VARIANT (tci.known_current_type);
 	  offset = tci.known_current_offset;
 	  maybe_in_construction = false;
 	  maybe_derived_type = false;
@@ -3044,7 +3142,7 @@ ipa_polymorphic_call_context::get_dynami
       else if (!speculative_outer_type
 	       || speculative_maybe_derived_type)
 	{
-	  speculative_outer_type = tci.known_current_type;
+	  speculative_outer_type = TYPE_MAIN_VARIANT (tci.known_current_type);
 	  speculative_offset = tci.known_current_offset;
 	  speculative_maybe_derived_type = false;
 	  if (dump_file)
@@ -3052,7 +3150,11 @@ ipa_polymorphic_call_context::get_dynami
 	}
     }
   else if (dump_file)
-    fprintf (dump_file, "  Found multiple types.\n");
+    {
+      fprintf (dump_file, "  Found multiple types%s%s\n",
+	       function_entry_reached ? " (function entry reached)" : "",
+	       function_entry_reached ? " (multiple types encountered)" : "");
+    }
 
   return true;
 }


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