Patchwork [wide-int] Simplify mult-to-shift conversion

login
register
mail settings
Submitter Richard Sandiford
Date Nov. 9, 2013, 10:51 a.m.
Message ID <87li0xg858.fsf@talisman.default>
Download mbox | patch
Permalink /patch/289966/
State New
Headers show

Comments

Richard Sandiford - Nov. 9, 2013, 10:51 a.m.
Trunk expand_mult uses the following code to handle CONST_DOUBLEs:

	  /* If we are multiplying in DImode, it may still be a win
	     to try to work with shifts and adds.  */
	  if (CONST_DOUBLE_HIGH (scalar_op1) == 0
	      && (CONST_DOUBLE_LOW (scalar_op1) > 0
		  || (CONST_DOUBLE_LOW (scalar_op1) < 0
		      && EXACT_POWER_OF_2_OR_ZERO_P
			   (CONST_DOUBLE_LOW (scalar_op1)))))
	    {
	      coeff = CONST_DOUBLE_LOW (scalar_op1);
	      is_neg = false;
	    }
	  else if (CONST_DOUBLE_LOW (scalar_op1) == 0)
	    {
	      coeff = CONST_DOUBLE_HIGH (scalar_op1);
	      if (EXACT_POWER_OF_2_OR_ZERO_P (coeff))
		{
		  int shift = floor_log2 (coeff) + HOST_BITS_PER_WIDE_INT;
		  if (shift < HOST_BITS_PER_DOUBLE_INT - 1
		      || mode_bitsize <= HOST_BITS_PER_DOUBLE_INT)
		    return expand_shift (LSHIFT_EXPR, mode, op0,
					 shift, target, unsignedp);
		}
	      goto skip_synth;
	    }
	  else
	    goto skip_synth;

So the only non-power-of-2 case that avoids the skip_synth treatment
is a CONST_DOUBLE whose high HWI is 0 and whose low HWI is positive.
I think that's outdated, because even on trunk such constants should be
CONST_INTs instead.  They definitely are on wide-int.  So we only need
to handle the power-of-2 case.

The shift handling for that case can be much simpler.  We're dealing
with a constant in a particular mode, so if exact_log2 returns >= 0,
it's a valid shift for that mode.  No truncation or boundary checks
are needed.  wide ints really clean this up.

The same reasoning applies to simplify_binary_operation_1, except there
we're just saving a redundant upper bound test.

Tested on powerpc64-linux-gnu and by rerunning the assembly comparison.
OK to install?

Thanks,
Richard
Kenneth Zadeck - Nov. 9, 2013, 3:09 p.m.
On 11/09/2013 05:51 AM, Richard Sandiford wrote:
> Trunk expand_mult uses the following code to handle CONST_DOUBLEs:
>
> 	  /* If we are multiplying in DImode, it may still be a win
> 	     to try to work with shifts and adds.  */
> 	  if (CONST_DOUBLE_HIGH (scalar_op1) == 0
> 	      && (CONST_DOUBLE_LOW (scalar_op1) > 0
> 		  || (CONST_DOUBLE_LOW (scalar_op1) < 0
> 		      && EXACT_POWER_OF_2_OR_ZERO_P
> 			   (CONST_DOUBLE_LOW (scalar_op1)))))
> 	    {
> 	      coeff = CONST_DOUBLE_LOW (scalar_op1);
> 	      is_neg = false;
> 	    }
> 	  else if (CONST_DOUBLE_LOW (scalar_op1) == 0)
> 	    {
> 	      coeff = CONST_DOUBLE_HIGH (scalar_op1);
> 	      if (EXACT_POWER_OF_2_OR_ZERO_P (coeff))
> 		{
> 		  int shift = floor_log2 (coeff) + HOST_BITS_PER_WIDE_INT;
> 		  if (shift < HOST_BITS_PER_DOUBLE_INT - 1
> 		      || mode_bitsize <= HOST_BITS_PER_DOUBLE_INT)
> 		    return expand_shift (LSHIFT_EXPR, mode, op0,
> 					 shift, target, unsignedp);
> 		}
> 	      goto skip_synth;
> 	    }
> 	  else
> 	    goto skip_synth;
>
> So the only non-power-of-2 case that avoids the skip_synth treatment
> is a CONST_DOUBLE whose high HWI is 0 and whose low HWI is positive.
> I think that's outdated, because even on trunk such constants should be
> CONST_INTs instead.  They definitely are on wide-int.  So we only need
> to handle the power-of-2 case.
>
> The shift handling for that case can be much simpler.  We're dealing
> with a constant in a particular mode, so if exact_log2 returns >= 0,
> it's a valid shift for that mode.  No truncation or boundary checks
> are needed.  wide ints really clean this up.
>
> The same reasoning applies to simplify_binary_operation_1, except there
> we're just saving a redundant upper bound test.
>
> Tested on powerpc64-linux-gnu and by rerunning the assembly comparison.
> OK to install?
>
> Thanks,
> Richard
>
>
> Index: gcc/expmed.c
> ===================================================================
> --- gcc/expmed.c	2013-11-09 10:50:12.836314443 +0000
> +++ gcc/expmed.c	2013-11-09 10:50:37.333494113 +0000
> @@ -3075,30 +3075,11 @@ expand_mult (enum machine_mode mode, rtx
>         else if (CONST_DOUBLE_AS_INT_P (scalar_op1))
>   #endif
>   	{
> -	  int p = GET_MODE_PRECISION (mode);
>   	  int shift = wi::exact_log2 (std::make_pair (scalar_op1, mode));
> -	  /* Perfect power of 2.  */
> -	  is_neg = false;
> +	  /* Perfect power of 2 (other than 1, which is handled above).  */
>   	  if (shift > 0)
> -	    {
> -	      /* Do the shift count trucation against the bitsize, not
> -		 the precision.  See the comment above
> -		 wide-int.c:trunc_shift for details.  */
> -	      if (SHIFT_COUNT_TRUNCATED)
> -		shift &= GET_MODE_BITSIZE (mode) - 1;
> -	      /* We could consider adding just a move of 0 to target
> -		 if the shift >= p  */
> -	      if (shift < p)
> -		return expand_shift (LSHIFT_EXPR, mode, op0,
> -				     shift, target, unsignedp);
> -	      /* Any positive number that fits in a word.  */
> -	      coeff = CONST_WIDE_INT_ELT (scalar_op1, 0);
> -	    }
> -	  else if (wi::sign_mask (std::make_pair (scalar_op1, mode)) == 0)
> -	    {
> -	      /* Any positive number that fits in a word.  */
> -	      coeff = CONST_WIDE_INT_ELT (scalar_op1, 0);
> -	    }
> +	    return expand_shift (LSHIFT_EXPR, mode, op0,
> +				 shift, target, unsignedp);
>   	  else
>   	    goto skip_synth;
>   	}
> Index: gcc/simplify-rtx.c
> ===================================================================
> --- gcc/simplify-rtx.c	2013-11-09 10:50:12.836314443 +0000
> +++ gcc/simplify-rtx.c	2013-11-09 10:50:14.046323332 +0000
> @@ -2399,7 +2399,7 @@ simplify_binary_operation_1 (enum rtx_co
>         if (CONST_SCALAR_INT_P (trueop1))
>   	{
>   	  val = wi::exact_log2 (std::make_pair (trueop1, mode));
> -	  if (val >= 0 && val < GET_MODE_BITSIZE (mode))
> +	  if (val >= 0)
>   	    return simplify_gen_binary (ASHIFT, mode, op0, GEN_INT (val));
>   	}
>   
looks good to me.

kenny

Patch

Index: gcc/expmed.c
===================================================================
--- gcc/expmed.c	2013-11-09 10:50:12.836314443 +0000
+++ gcc/expmed.c	2013-11-09 10:50:37.333494113 +0000
@@ -3075,30 +3075,11 @@  expand_mult (enum machine_mode mode, rtx
       else if (CONST_DOUBLE_AS_INT_P (scalar_op1))
 #endif
 	{
-	  int p = GET_MODE_PRECISION (mode);
 	  int shift = wi::exact_log2 (std::make_pair (scalar_op1, mode));
-	  /* Perfect power of 2.  */
-	  is_neg = false;
+	  /* Perfect power of 2 (other than 1, which is handled above).  */
 	  if (shift > 0)
-	    {
-	      /* Do the shift count trucation against the bitsize, not
-		 the precision.  See the comment above
-		 wide-int.c:trunc_shift for details.  */
-	      if (SHIFT_COUNT_TRUNCATED)
-		shift &= GET_MODE_BITSIZE (mode) - 1;
-	      /* We could consider adding just a move of 0 to target
-		 if the shift >= p  */
-	      if (shift < p)
-		return expand_shift (LSHIFT_EXPR, mode, op0, 
-				     shift, target, unsignedp);
-	      /* Any positive number that fits in a word.  */
-	      coeff = CONST_WIDE_INT_ELT (scalar_op1, 0);
-	    }
-	  else if (wi::sign_mask (std::make_pair (scalar_op1, mode)) == 0)
-	    {
-	      /* Any positive number that fits in a word.  */
-	      coeff = CONST_WIDE_INT_ELT (scalar_op1, 0);
-	    }
+	    return expand_shift (LSHIFT_EXPR, mode, op0,
+				 shift, target, unsignedp);
 	  else
 	    goto skip_synth;
 	}
Index: gcc/simplify-rtx.c
===================================================================
--- gcc/simplify-rtx.c	2013-11-09 10:50:12.836314443 +0000
+++ gcc/simplify-rtx.c	2013-11-09 10:50:14.046323332 +0000
@@ -2399,7 +2399,7 @@  simplify_binary_operation_1 (enum rtx_co
       if (CONST_SCALAR_INT_P (trueop1))
 	{
 	  val = wi::exact_log2 (std::make_pair (trueop1, mode));
-	  if (val >= 0 && val < GET_MODE_BITSIZE (mode))
+	  if (val >= 0)
 	    return simplify_gen_binary (ASHIFT, mode, op0, GEN_INT (val));
 	}