Re: Allow matching of side effects in operand_equal_p

[Richard Biener <rguenther at suse dot de>]

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).

Thanks,
Richard.

> Bootstrapped/regtested x86_64-linux, OK?
> 
> 	* tree-core.h (OEP_MATCH_SIDE_EFFECTS): New.
> 	* tree-ssa-tail-merge.c (gimple_operand_equal_value_p): Pass
> 	OEP_MATCH_SIDE_EFFECTS
> 	* fold-const.c (operand_equal_p): Support OEP_MATCH_SIDE_EFFECTS.
> Index: tree-core.h
> ===================================================================
> --- tree-core.h	(revision 229263)
> +++ tree-core.h	(working copy)
> @@ -732,7 +732,8 @@ enum operand_equal_flag {
>    OEP_ONLY_CONST = 1,
>    OEP_PURE_SAME = 2,
>    OEP_CONSTANT_ADDRESS_OF = 4,
> -  OEP_ADDRESS_OF = 8
> +  OEP_ADDRESS_OF = 8,
> +  OEP_MATCH_SIDE_EFFECTS = 16
>  };
>  
>  /* Enum and arrays used for tree allocation stats.
> Index: tree-ssa-tail-merge.c
> ===================================================================
> --- tree-ssa-tail-merge.c	(revision 229263)
> +++ tree-ssa-tail-merge.c	(working copy)
> @@ -1091,7 +1091,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);
> Index: fold-const.c
> ===================================================================
> --- fold-const.c	(revision 229494)
> +++ fold-const.c	(working copy)
> @@ -2657,8 +2657,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;
> @@ -2685,7 +2684,12 @@ combine_comparisons (location_t loc,
>     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.  */
> +   further ignore any side effects on SAVE_EXPRs then.
> +
> +   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).  */
>  
>  int
>  operand_equal_p (const_tree arg0, const_tree arg1, unsigned int flags)
> @@ -2814,7 +2818,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_CONSTANT_ADDRESS_OF | OEP_MATCH_SIDE_EFFECTS))
>  	  || (! TREE_SIDE_EFFECTS (arg0) && ! TREE_SIDE_EFFECTS (arg1))))
>      return 1;
>  
> @@ -2936,7 +2940,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_CONSTANT_ADDRESS_OF | OEP_MATCH_SIDE_EFFECTS)) == 0
>  	  && (TREE_SIDE_EFFECTS (arg0)
>  	      || TREE_SIDE_EFFECTS (arg1)))
>  	return 0;
> @@ -3097,15 +3101,16 @@ operand_equal_p (const_tree arg0, const_
>  		return 0;
>  	    }
>  
> -	  {
> -	    unsigned int cef = call_expr_flags (arg0);
> -	    if (flags & OEP_PURE_SAME)
> -	      cef &= ECF_CONST | ECF_PURE;
> -	    else
> -	      cef &= ECF_CONST;
> -	    if (!cef)
> -	      return 0;
> -	  }
> +	  if (!(flags & OEP_MATCH_SIDE_EFFECTS))
> +	    {
> +	      unsigned int cef = call_expr_flags (arg0);
> +	      if (flags & OEP_PURE_SAME)
> +		cef &= ECF_CONST | ECF_PURE;
> +	      else
> +		cef &= ECF_CONST;
> +	      if (!cef)
> +		return 0;
> +	    }
>  
>  	  /* Now see if all the arguments are the same.  */
>  	  {
> 
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)