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]

Re: [PATCH] Improve rotate fold-const pattern matching (PR target/82498)


On Thu, 12 Oct 2017, Jakub Jelinek wrote:

> Hi!
> 
> Marc in the PR mentioned that it is not really good that the recommended
> rotate pattern is recognized only during forwprop1 and later, which is after
> einline and that inlining or early opts could have changed stuff too much so
> that we wouldn't recogize it anymore.

Hmm, but the only thing functions see is inlining early optimized bodies
into them and then constant propagation performed, so I don't see how
we could confuse the pattern in a way to be indetectable.  Also
early inlining is performed on early optimized bodies so cost metrics
see rotates, not the unrecognized form.

> The following patch handles that pattern in fold_binary_loc too, and while
> I've touched it, it cleans a lot of ugliness/duplication in that code.
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

Still looks like an improvement, thus ok.

Thanks,
Richard.

> 2017-10-12  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR target/82498
> 	* fold-const.c (fold_binary_loc) <bit_rotate>: Code cleanups,
> 	instead of handling MINUS_EXPR twice (once for each argument),
> 	canonicalize operand order and handle just once, use rtype where
> 	possible.  Handle (A << B) | (A >> (-B & (Z - 1))).
> 
> 	* gcc.dg/tree-ssa/pr82498.c: New test.
> 
> --- gcc/fold-const.c.jj	2017-10-11 22:37:51.000000000 +0200
> +++ gcc/fold-const.c	2017-10-12 13:17:45.360589554 +0200
> @@ -9429,7 +9429,10 @@ fold_binary_loc (location_t loc,
>        /* (A << C1) + (A >> C2) if A is unsigned and C1+C2 is the size of A
>  	 is a rotate of A by C1 bits.  */
>        /* (A << B) + (A >> (Z - B)) if A is unsigned and Z is the size of A
> -	 is a rotate of A by B bits.  */
> +	 is a rotate of A by B bits.
> +	 Similarly for (A << B) | (A >> (-B & C3)) where C3 is Z-1,
> +	 though in this case CODE must be | and not + or ^, otherwise
> +	 it doesn't return A when B is 0.  */
>        {
>  	enum tree_code code0, code1;
>  	tree rtype;
> @@ -9447,25 +9450,32 @@ fold_binary_loc (location_t loc,
>  		== GET_MODE_UNIT_PRECISION (TYPE_MODE (rtype))))
>  	  {
>  	    tree tree01, tree11;
> +	    tree orig_tree01, orig_tree11;
>  	    enum tree_code code01, code11;
>  
> -	    tree01 = TREE_OPERAND (arg0, 1);
> -	    tree11 = TREE_OPERAND (arg1, 1);
> +	    tree01 = orig_tree01 = TREE_OPERAND (arg0, 1);
> +	    tree11 = orig_tree11 = TREE_OPERAND (arg1, 1);
>  	    STRIP_NOPS (tree01);
>  	    STRIP_NOPS (tree11);
>  	    code01 = TREE_CODE (tree01);
>  	    code11 = TREE_CODE (tree11);
> +	    if (code11 != MINUS_EXPR
> +		&& (code01 == MINUS_EXPR || code01 == BIT_AND_EXPR))
> +	      {
> +		std::swap (code0, code1);
> +		std::swap (code01, code11);
> +		std::swap (tree01, tree11);
> +		std::swap (orig_tree01, orig_tree11);
> +	      }
>  	    if (code01 == INTEGER_CST
>  		&& code11 == INTEGER_CST
>  		&& (wi::to_widest (tree01) + wi::to_widest (tree11)
> -		    == element_precision (TREE_TYPE (TREE_OPERAND (arg0, 0)))))
> +		    == element_precision (rtype)))
>  	      {
>  		tem = build2_loc (loc, LROTATE_EXPR,
> -				  TREE_TYPE (TREE_OPERAND (arg0, 0)),
> -				  TREE_OPERAND (arg0, 0),
> +				  rtype, TREE_OPERAND (arg0, 0),
>  				  code0 == LSHIFT_EXPR
> -				  ? TREE_OPERAND (arg0, 1)
> -				  : TREE_OPERAND (arg1, 1));
> +				  ? orig_tree01 : orig_tree11);
>  		return fold_convert_loc (loc, type, tem);
>  	      }
>  	    else if (code11 == MINUS_EXPR)
> @@ -9477,39 +9487,37 @@ fold_binary_loc (location_t loc,
>  		STRIP_NOPS (tree111);
>  		if (TREE_CODE (tree110) == INTEGER_CST
>  		    && 0 == compare_tree_int (tree110,
> -					      element_precision
> -					      (TREE_TYPE (TREE_OPERAND
> -							  (arg0, 0))))
> +					      element_precision (rtype))
>  		    && operand_equal_p (tree01, tree111, 0))
> -		  return
> -		    fold_convert_loc (loc, type,
> -				      build2 ((code0 == LSHIFT_EXPR
> -					       ? LROTATE_EXPR
> -					       : RROTATE_EXPR),
> -					      TREE_TYPE (TREE_OPERAND (arg0, 0)),
> -					      TREE_OPERAND (arg0, 0),
> -					      TREE_OPERAND (arg0, 1)));
> +		  {
> +		    tem = build2_loc (loc, (code0 == LSHIFT_EXPR
> +					    ? LROTATE_EXPR : RROTATE_EXPR),
> +				      rtype, TREE_OPERAND (arg0, 0),
> +				      orig_tree01);
> +		    return fold_convert_loc (loc, type, tem);
> +		  }
>  	      }
> -	    else if (code01 == MINUS_EXPR)
> +	    else if (code == BIT_IOR_EXPR
> +		     && code11 == BIT_AND_EXPR
> +		     && pow2p_hwi (element_precision (rtype)))
>  	      {
> -		tree tree010, tree011;
> -		tree010 = TREE_OPERAND (tree01, 0);
> -		tree011 = TREE_OPERAND (tree01, 1);
> -		STRIP_NOPS (tree010);
> -		STRIP_NOPS (tree011);
> -		if (TREE_CODE (tree010) == INTEGER_CST
> -		    && 0 == compare_tree_int (tree010,
> -					      element_precision
> -					      (TREE_TYPE (TREE_OPERAND
> -							  (arg0, 0))))
> -		    && operand_equal_p (tree11, tree011, 0))
> -		    return fold_convert_loc
> -		      (loc, type,
> -		       build2 ((code0 != LSHIFT_EXPR
> -				? LROTATE_EXPR
> -				: RROTATE_EXPR),
> -			       TREE_TYPE (TREE_OPERAND (arg0, 0)),
> -			       TREE_OPERAND (arg0, 0), TREE_OPERAND (arg1, 1)));
> +		tree tree110, tree111;
> +		tree110 = TREE_OPERAND (tree11, 0);
> +		tree111 = TREE_OPERAND (tree11, 1);
> +		STRIP_NOPS (tree110);
> +		STRIP_NOPS (tree111);
> +		if (TREE_CODE (tree110) == NEGATE_EXPR
> +		    && TREE_CODE (tree111) == INTEGER_CST
> +		    && 0 == compare_tree_int (tree111,
> +					      element_precision (rtype) - 1)
> +		    && operand_equal_p (tree01, TREE_OPERAND (tree110, 0), 0))
> +		  {
> +		    tem = build2_loc (loc, (code0 == LSHIFT_EXPR
> +					    ? LROTATE_EXPR : RROTATE_EXPR),
> +				      rtype, TREE_OPERAND (arg0, 0),
> +				      orig_tree01);
> +		    return fold_convert_loc (loc, type, tem);
> +		  }
>  	      }
>  	  }
>        }
> --- gcc/testsuite/gcc.dg/tree-ssa/pr82498.c.jj	2017-10-12 13:14:27.234991970 +0200
> +++ gcc/testsuite/gcc.dg/tree-ssa/pr82498.c	2017-10-12 13:13:45.000000000 +0200
> @@ -0,0 +1,53 @@
> +/* PR target/82498 */
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -fdump-tree-original" } */
> +/* { dg-final { scan-tree-dump-times "x r<< y" 4 "original" { target int32 } } } */
> +/* { dg-final { scan-tree-dump-times "x r>> y" 4 "original" { target int32 } } } */
> +
> +unsigned
> +f1 (unsigned x, int y)
> +{
> +  return (x << y) | (x >> (__CHAR_BIT__ * __SIZEOF_INT__ - y));
> +}
> +
> +unsigned
> +f2 (unsigned x, int y)
> +{
> +  return (x << y) | (x >> (-y & (__CHAR_BIT__ * __SIZEOF_INT__ - 1)));
> +}
> +
> +unsigned
> +f3 (unsigned x, int y)
> +{
> +  return (x >> y) | (x << (__CHAR_BIT__ * __SIZEOF_INT__ - y));
> +}
> +
> +unsigned
> +f4 (unsigned x, int y)
> +{
> +  return (x >> y) | (x << (-y & (__CHAR_BIT__ * __SIZEOF_INT__ - 1)));
> +}
> +
> +unsigned
> +f5 (unsigned x, int y)
> +{
> +  return (x >> (__CHAR_BIT__ * __SIZEOF_INT__ - y)) | (x << y);
> +}
> +
> +unsigned
> +f6 (unsigned x, int y)
> +{
> +  return (x >> (-y & (__CHAR_BIT__ * __SIZEOF_INT__ - 1))) | (x << y);
> +}
> +
> +unsigned
> +f7 (unsigned x, int y)
> +{
> +  return (x << (__CHAR_BIT__ * __SIZEOF_INT__ - y)) | (x >> y);
> +}
> +
> +unsigned
> +f8 (unsigned x, int y)
> +{
> +  return (x << (-y & (__CHAR_BIT__ * __SIZEOF_INT__ - 1))) | (x >> y);
> +}
> 
> 	Jakub
> 
> 

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


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