diff mbox series

Fix two issues I introduced in operand_equal_p

Message ID 20201119192152.GH90289@kam.mff.cuni.cz
State New
Headers show
Series Fix two issues I introduced in operand_equal_p | expand

Commit Message

Jan Hubicka Nov. 19, 2020, 7:21 p.m. UTC 
Hi,
doing some further testing and analysis of icf miscompares I noticed tat
my change for hadling OEP_ADDRESS_OF of COMPONENT_REF had last minute
chnage that made it not effective, since flag is cleared before the
conditional.  After some exprimenting it seem cleanest to just use
temporary bool.

Other problem is that obj-C++ produces OBJ_TYPE_REFs that are referring
to something else than class types. obj_type_ref_class asserts for that
since one is expected to use virutal_method_call_p predicate first.
It would be nice to make obj-C++ either produce standard OBJ_TYEP_REFs
or use different code for that, but that is for another day.

I apologize for both - clearly need a break which I will do. Fortunately
it seems that ICF issues tracked in PR92535 are almost resolved.

lto-bootstraped and regtested x86_64-linux, comitted.

Honza

	* fold-const.c (operand_compare::operand_equal_p): Fix thinko in
	COMPONENT_REF handling and guard types_same_for_odr by
	virtual_method_call_p.
	(operand_compare::hash_operand): Likewise.
diff mbox series

Patch

diff --git a/gcc/fold-const.c b/gcc/fold-const.c
index 820b08d26fd..1bce9e72c1d 100644
--- a/gcc/fold-const.c
+++ b/gcc/fold-const.c
@@ -3314,30 +3314,34 @@  operand_compare::operand_equal_p (const_tree arg0, const_tree arg1,
 	     may be NULL when we're called to compare MEM_EXPRs.  */
 	  if (!OP_SAME_WITH_NULL (0))
 	    return false;
-	  /* Most of time we only need to compare FIELD_DECLs for equality.
-	     However when determining address look into actual offsets.
-	     These may match for unions and unshared record types.  */
-	  flags &= ~OEP_ADDRESS_OF;
-	  if (!OP_SAME (1))
-	    {
-	      if (flags & OEP_ADDRESS_OF)
-		{
-		  if (TREE_OPERAND (arg0, 2)
-		      || TREE_OPERAND (arg1, 2))
-		    return OP_SAME_WITH_NULL (2);
-		  tree field0 = TREE_OPERAND (arg0, 1);
-		  tree field1 = TREE_OPERAND (arg1, 1);
-
-		  if (!operand_equal_p (DECL_FIELD_OFFSET (field0),
-					DECL_FIELD_OFFSET (field1), flags)
-		      || !operand_equal_p (DECL_FIELD_BIT_OFFSET (field0),
-					   DECL_FIELD_BIT_OFFSET (field1),
-					   flags))
-		    return false;
-		}
-	      else
-		return false;
-	    }
+	  {
+	    bool compare_address = flags & OEP_ADDRESS_OF;
+
+	    /* Most of time we only need to compare FIELD_DECLs for equality.
+	       However when determining address look into actual offsets.
+	       These may match for unions and unshared record types.  */
+	    flags &= ~OEP_ADDRESS_OF;
+	    if (!OP_SAME (1))
+	      {
+		if (compare_address)
+		  {
+		    if (TREE_OPERAND (arg0, 2)
+			|| TREE_OPERAND (arg1, 2))
+		      return OP_SAME_WITH_NULL (2);
+		    tree field0 = TREE_OPERAND (arg0, 1);
+		    tree field1 = TREE_OPERAND (arg1, 1);
+
+		    if (!operand_equal_p (DECL_FIELD_OFFSET (field0),
+					  DECL_FIELD_OFFSET (field1), flags)
+			|| !operand_equal_p (DECL_FIELD_BIT_OFFSET (field0),
+					     DECL_FIELD_BIT_OFFSET (field1),
+					     flags))
+		      return false;
+		  }
+		else
+		  return false;
+	      }
+	  }
 	  return OP_SAME_WITH_NULL (2);
 
 	case BIT_FIELD_REF:
@@ -3436,8 +3440,11 @@  operand_compare::operand_equal_p (const_tree arg0, const_tree arg1,
 	if (!operand_equal_p (OBJ_TYPE_REF_OBJECT (arg0),
 			      OBJ_TYPE_REF_OBJECT (arg1), flags))
 	  return false;
-	if (!types_same_for_odr (obj_type_ref_class (arg0),
-				 obj_type_ref_class (arg1)))
+	if (virtual_method_call_p (arg0) != virtual_method_call_p (arg1))
+	  return false;
+	if (virtual_method_call_p (arg0)
+	    && !types_same_for_odr (obj_type_ref_class (arg0),
+				    obj_type_ref_class (arg1)))
 	  return false;
 	return true;
 
@@ -3866,6 +3873,8 @@  operand_compare::hash_operand (const_tree t, inchash::hash &hstate,
 	      flags &= ~OEP_ADDRESS_OF;
 	      inchash::add_expr (OBJ_TYPE_REF_TOKEN (t), hstate, flags);
 	      inchash::add_expr (OBJ_TYPE_REF_OBJECT (t), hstate, flags);
+	      if (!virtual_method_call_p (t))
+		return;
 	      if (tree c = obj_type_ref_class (t))
 		{
 		  c = TYPE_NAME (TYPE_MAIN_VARIANT (c));