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

Message ID 87li0xg858.fsf@talisman.default New show

Commit Message

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. | #1
```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));
}

```