diff mbox

Allow matching of side effects in operand_equal_p

Message ID 20151106195114.GB27859@kam.mff.cuni.cz
State New
Headers show

Commit Message

Jan Hubicka Nov. 6, 2015, 7:51 p.m. UTC 
> On Mon, 2 Nov 2015, Jan Hubicka wrote:
> 
> > Hi,
> > this patch adds OEP_MATCH_SIDE_EFFECT to tell operand_equal_p that the two
> > operands compared are from different code paths and thus they can be matched
> > even if they have side effects.
> > 
> > I.e. 
> > 
> > volatile int a;
> > 
> > if (a==a)
> > 
> > has two reads and thus can't be optimized (I guess), while
> > 
> > b?a:a
> > 
> > can be optimized.  OEP_MATCH_SIDE_EFFECTS will be needed by ipa-icf.  I changed
> > gimple_operand_equal_value_p in tail-merge to use it.  I do not know how to
> > make the folding of ?: happen, since there seems to be no way to specify flags
> > to match_operand in match.pd. This makes it hard to construct a testcase - all
> > my attempts to trick tree-ssa-tailmerge to merge something that involve volatile
> > operands failed, because it uses GVN to compare and GVN punt on these.
> > 
> > I also wonder if we can drop OEP_CONSTANT_ADDRESS_OF. The only difference is that
> > OEP_MATCH_SIDE_EFFECTS permits function calls.  I think those should not appear
> > in constant addresses.
> 
> But you don't even need the call case for tailmerge or ICF as we don't
> have CALL_EXPRs in GIMPLE.  No?
> 
> And you mean the difference of OEP_CONSTANT_ADDRESS_OF to
> OEP_ADDRESS_OF | OEP_MATCH_SIDE_EFFECTS then, right?
> 
> Ok if you go forward with that and do not change the CALL_EXPR case
> for now (you may add a FIXME comment).

Thank, this is variant I comitted which drops OEP_CONSTANT_ADDRESS_OF. Now I
finally get what OEP_CONSTANT_ADDRESS_OF is for: we want to compare
&volatilve_var == &volatile_var Obvoiusly when doing OEP_ADDRESS_OF, the
TREE_SIDE_EFFECTS flag is not really informative in general.  I suppose we
could match &volatile_array[b] == &volatile_array[b] even if the address is no
longer constant.  Perhaps it is afe to ignore TREE_SIDE_EFFECTS for
OEP_ADDRESS_OF in general?  I can not see how address calculation can trigger
side effects in other way than computing the offset (ie. volatile_var[func()])
but then the offset calculation is a normal expression and we do not get
OEP_ADDRESS_OF.

Honza

	* tree-core.h (size_type_kind): Remove OEP_CONSTANT_ADDRESS_OF and
	add OEP_MATCH_SIDE_EFFECTS.
	* fold-const.c (operand_equal_p): Update documentation; handle
	OEP_MATCH_SIDE_EFFECTS.
	* tree-ssa-tail-merge.c (gimple_operand_equal_value_p): Use
	OEP_MATCH_SIDE_EFFECTS.
diff mbox

Patch

Index: tree-core.h
===================================================================
--- tree-core.h	(revision 229858)
+++ tree-core.h	(working copy)
@@ -737,7 +737,7 @@  enum size_type_kind {
 enum operand_equal_flag {
   OEP_ONLY_CONST = 1,
   OEP_PURE_SAME = 2,
-  OEP_CONSTANT_ADDRESS_OF = 4,
+  OEP_MATCH_SIDE_EFFECTS = 4,
   OEP_ADDRESS_OF = 8
 };
 
Index: fold-const.c
===================================================================
--- fold-const.c	(revision 229858)
+++ fold-const.c	(working copy)
@@ -2649,8 +2649,7 @@  combine_comparisons (location_t loc,
 }
 
 /* Return nonzero if two operands (typically of the same tree node)
-   are necessarily equal.  If either argument has side-effects this
-   function returns zero.  FLAGS modifies behavior as follows:
+   are necessarily equal. FLAGS modifies behavior as follows:
 
    If OEP_ONLY_CONST is set, only return nonzero for constants.
    This function tests whether the operands are indistinguishable;
@@ -2675,9 +2674,14 @@  combine_comparisons (location_t loc,
    to ensure that global memory is unchanged in between.
 
    If OEP_ADDRESS_OF is set, we are actually comparing addresses of objects,
-   not values of expressions.  OEP_CONSTANT_ADDRESS_OF in addition to
-   OEP_ADDRESS_OF is used for ADDR_EXPR with TREE_CONSTANT flag set and we
-   further ignore any side effects on SAVE_EXPRs then.  */
+   not values of expressions.
+
+   Unless OEP_MATCH_SIDE_EFFECTS is set, the function returns false on
+   any operand with side effect.  This is unnecesarily conservative in the
+   case we know that arg0 and arg1 are in disjoint code paths (such as in
+   ?: operator).  In addition OEP_MATCH_SIDE_EFFECTS is used when comparing
+   addresses with TREE_CONSTANT flag set so we know that &var == &var
+   even if var is volatile.  */
 
 int
 operand_equal_p (const_tree arg0, const_tree arg1, unsigned int flags)
@@ -2698,9 +2702,8 @@  operand_equal_p (const_tree arg0, const_
   if (TREE_CODE (arg0) == INTEGER_CST && TREE_CODE (arg1) == INTEGER_CST)
     {
       /* Address of INTEGER_CST is not defined; check that we did not forget
-	 to drop the OEP_ADDRESS_OF/OEP_CONSTANT_ADDRESS_OF flags.  */
-      gcc_checking_assert (!(flags
-			     & (OEP_ADDRESS_OF | OEP_CONSTANT_ADDRESS_OF)));
+	 to drop the OEP_ADDRESS_OF flags.  */
+      gcc_checking_assert (!(flags & OEP_ADDRESS_OF));
       return tree_int_cst_equal (arg0, arg1);
     }
 
@@ -2806,7 +2809,7 @@  operand_equal_p (const_tree arg0, const_
      they are necessarily equal as well.  */
   if (arg0 == arg1 && ! (flags & OEP_ONLY_CONST)
       && (TREE_CODE (arg0) == SAVE_EXPR
-	  || (flags & OEP_CONSTANT_ADDRESS_OF)
+	  || (flags & OEP_MATCH_SIDE_EFFECTS)
 	  || (! TREE_SIDE_EFFECTS (arg0) && ! TREE_SIDE_EFFECTS (arg1))))
     return 1;
 
@@ -2865,11 +2868,10 @@  operand_equal_p (const_tree arg0, const_
 			      TREE_STRING_LENGTH (arg0)));
 
       case ADDR_EXPR:
-	gcc_checking_assert (!(flags
-			       & (OEP_ADDRESS_OF | OEP_CONSTANT_ADDRESS_OF)));
+	gcc_checking_assert (!(flags & OEP_ADDRESS_OF));
 	return operand_equal_p (TREE_OPERAND (arg0, 0), TREE_OPERAND (arg1, 0),
 				flags | OEP_ADDRESS_OF
-				| OEP_CONSTANT_ADDRESS_OF);
+				| OEP_MATCH_SIDE_EFFECTS);
       case CONSTRUCTOR:
 	/* In GIMPLE empty constructors are allowed in initializers of
 	   aggregates.  */
@@ -2928,7 +2930,7 @@  operand_equal_p (const_tree arg0, const_
       /* If either of the pointer (or reference) expressions we are
 	 dereferencing contain a side effect, these cannot be equal,
 	 but their addresses can be.  */
-      if ((flags & OEP_CONSTANT_ADDRESS_OF) == 0
+      if ((flags & OEP_MATCH_SIDE_EFFECTS) == 0
 	  && (TREE_SIDE_EFFECTS (arg0)
 	      || TREE_SIDE_EFFECTS (arg1)))
 	return 0;
@@ -2936,11 +2938,11 @@  operand_equal_p (const_tree arg0, const_
       switch (TREE_CODE (arg0))
 	{
 	case INDIRECT_REF:
-	  if (!(flags & (OEP_ADDRESS_OF | OEP_CONSTANT_ADDRESS_OF))
+	  if (!(flags & OEP_ADDRESS_OF)
 	      && (TYPE_ALIGN (TREE_TYPE (arg0))
 		  != TYPE_ALIGN (TREE_TYPE (arg1))))
 	    return 0;
-	  flags &= ~(OEP_CONSTANT_ADDRESS_OF|OEP_ADDRESS_OF);
+	  flags &= ~OEP_ADDRESS_OF;
 	  return OP_SAME (0);
 
 	case REALPART_EXPR:
@@ -2950,7 +2952,7 @@  operand_equal_p (const_tree arg0, const_
 
 	case TARGET_MEM_REF:
 	case MEM_REF:
-	  if (!(flags & (OEP_ADDRESS_OF | OEP_CONSTANT_ADDRESS_OF)))
+	  if (!(flags & OEP_ADDRESS_OF))
 	    {
 	      /* Require equal access sizes */
 	      if (TYPE_SIZE (TREE_TYPE (arg0)) != TYPE_SIZE (TREE_TYPE (arg1))
@@ -2975,7 +2977,7 @@  operand_equal_p (const_tree arg0, const_
 		 != TYPE_ALIGN (TREE_TYPE (arg1)))
 		return 0;
 	    }
-	  flags &= ~(OEP_CONSTANT_ADDRESS_OF|OEP_ADDRESS_OF);
+	  flags &= ~OEP_ADDRESS_OF;
 	  return (OP_SAME (0) && OP_SAME (1)
 		  /* TARGET_MEM_REF require equal extra operands.  */
 		  && (TREE_CODE (arg0) != TARGET_MEM_REF
@@ -2990,7 +2992,7 @@  operand_equal_p (const_tree arg0, const_
 	     may have different types but same value here.  */
 	  if (!OP_SAME (0))
 	    return 0;
-	  flags &= ~(OEP_CONSTANT_ADDRESS_OF|OEP_ADDRESS_OF);
+	  flags &= ~OEP_ADDRESS_OF;
 	  return ((tree_int_cst_equal (TREE_OPERAND (arg0, 1),
 				       TREE_OPERAND (arg1, 1))
 		   || OP_SAME (1))
@@ -3003,13 +3005,13 @@  operand_equal_p (const_tree arg0, const_
 	  if (!OP_SAME_WITH_NULL (0)
 	      || !OP_SAME (1))
 	    return 0;
-	  flags &= ~(OEP_CONSTANT_ADDRESS_OF|OEP_ADDRESS_OF);
+	  flags &= ~OEP_ADDRESS_OF;
 	  return OP_SAME_WITH_NULL (2);
 
 	case BIT_FIELD_REF:
 	  if (!OP_SAME (0))
 	    return 0;
-	  flags &= ~(OEP_CONSTANT_ADDRESS_OF|OEP_ADDRESS_OF);
+	  flags &= ~OEP_ADDRESS_OF;
 	  return OP_SAME (1) && OP_SAME (2);
 
 	default:
@@ -3021,9 +3023,7 @@  operand_equal_p (const_tree arg0, const_
 	{
 	case ADDR_EXPR:
 	  /* Be sure we pass right ADDRESS_OF flag.  */
-	  gcc_checking_assert (!(flags
-				 & (OEP_ADDRESS_OF
-				    | OEP_CONSTANT_ADDRESS_OF)));
+	  gcc_checking_assert (!(flags & OEP_ADDRESS_OF));
 	  return operand_equal_p (TREE_OPERAND (arg0, 0),
 				  TREE_OPERAND (arg1, 0),
 				  flags | OEP_ADDRESS_OF);
@@ -3089,6 +3089,7 @@  operand_equal_p (const_tree arg0, const_
 		return 0;
 	    }
 
+	  /* FIXME: We could skip this test for OEP_MATCH_SIDE_EFFECTS.  */
 	  {
 	    unsigned int cef = call_expr_flags (arg0);
 	    if (flags & OEP_PURE_SAME)
Index: tree-ssa-tail-merge.c
===================================================================
--- tree-ssa-tail-merge.c	(revision 229858)
+++ tree-ssa-tail-merge.c	(working copy)
@@ -1081,7 +1081,7 @@  gimple_operand_equal_value_p (tree t1, t
       || t2 == NULL_TREE)
     return false;
 
-  if (operand_equal_p (t1, t2, 0))
+  if (operand_equal_p (t1, t2, OEP_MATCH_SIDE_EFFECTS))
     return true;
 
   return gvn_uses_equal (t1, t2);