Constant fold/simplify SS_ASHIFT and US_ASHIFT in simplify-rtx.c

Message ID 001101d7c973$e5de92b0$b19bb810$@nextmovesoftware.com
State New
Headers
Series Constant fold/simplify SS_ASHIFT and US_ASHIFT in simplify-rtx.c |

Commit Message

Roger Sayle Oct. 25, 2021, 7:42 a.m. UTC
  This patch adds compile-time evaluation of signed saturating left shift
(SS_ASHIFT) and unsigned saturating left shift (US_ASHIFT) to simplify-rtx's
simplify_const_binary_operation.  US_ASHIFT saturates to the maximum
unsigned value on overflow (which occurs when the shift is greater than
the leading zero count), while SS_ASHIFT saturates on overflow to the
maximum signed value for positive arguments, and the minimum signed value
for negative arguments (which occurs when the shift count is greater than
the number of leading redundant sign bits, clrsb).  This suggests
some additional simplifications that this patch implements in
simplify_binary_operation_1; us_ashift:HI of 0xffff remains 0xffff
(much like any ashift of 0x0000 remains 0x0000), and ss_ashift:HI of
0x7fff remains 0x7ffff, and of 0x8000 remains 0x8000.

Conveniently the bfin backend provides instructions/built-ins that allow
this functionality to be tested.  The two functions below

short stest_sat_max() { return __builtin_bfin_shl_fr1x16(10000,8); }
short stest_sat_min() { return __builtin_bfin_shl_fr1x16(-10000,8); }

previously on bfin-elf with -O2 generated:

_stest_sat_max:
        nop;
        nop;
        R0 = 10000 (X);
        R0 = R0 << 8 (V,S);
        rts;

_stest_sat_min:
        nop;
        nop;
        R0 = -10000 (X);
        R0 = R0 << 8 (V,S);
        rts;

With this patch, bfin-elf now generates:

_stest_sat_max:
        nop;
        nop;
        nop;
        R0 = 32767 (X);
        rts;

_stest_sat_min:
        nop;
        nop;
        nop;
        R0 = -32768 (X);
        rts;

This patch has been tested on x86_64-pc-linux-gnu with make bootstrap and
make -k check with no new failures, and on a cross-compiler to bfin-elf
with no regressions.  Ok for mainline?


2021-10-25  Roger Sayle  <roger@nextmovesoftware.com>

gcc/ChangeLog
	* simplify-rtx (simplify_binary_operation_1) [SS_ASHIFT]: Simplify
	shifts of the mode's smin_value and smax_value when the bit count
	operand doesn't have side-effects.
	[US_ASHIFT]: Likewise, simplify shifts of the mode's umax_value
	when the bit count operand doesn't have side-effects.
	(simplify_const_binary_operation) [SS_ASHIFT, US_ASHIFT]: Perform
	compile-time evaluation of saturating left shifts with constant
	arguments.

gcc/testsuite/ChangeLog
	* gcc.target/bfin/ssashift-1.c: New test case.


Roger
--
  

Comments

Richard Sandiford Oct. 25, 2021, 11:38 a.m. UTC | #1
"Roger Sayle" <roger@nextmovesoftware.com> writes:
> This patch adds compile-time evaluation of signed saturating left shift
> (SS_ASHIFT) and unsigned saturating left shift (US_ASHIFT) to simplify-rtx's
> simplify_const_binary_operation.  US_ASHIFT saturates to the maximum
> unsigned value on overflow (which occurs when the shift is greater than
> the leading zero count), while SS_ASHIFT saturates on overflow to the
> maximum signed value for positive arguments, and the minimum signed value
> for negative arguments (which occurs when the shift count is greater than
> the number of leading redundant sign bits, clrsb).  This suggests
> some additional simplifications that this patch implements in
> simplify_binary_operation_1; us_ashift:HI of 0xffff remains 0xffff
> (much like any ashift of 0x0000 remains 0x0000), and ss_ashift:HI of
> 0x7fff remains 0x7ffff, and of 0x8000 remains 0x8000.
>
> Conveniently the bfin backend provides instructions/built-ins that allow
> this functionality to be tested.  The two functions below
>
> short stest_sat_max() { return __builtin_bfin_shl_fr1x16(10000,8); }
> short stest_sat_min() { return __builtin_bfin_shl_fr1x16(-10000,8); }
>
> previously on bfin-elf with -O2 generated:
>
> _stest_sat_max:
>         nop;
>         nop;
>         R0 = 10000 (X);
>         R0 = R0 << 8 (V,S);
>         rts;
>
> _stest_sat_min:
>         nop;
>         nop;
>         R0 = -10000 (X);
>         R0 = R0 << 8 (V,S);
>         rts;
>
> With this patch, bfin-elf now generates:
>
> _stest_sat_max:
>         nop;
>         nop;
>         nop;
>         R0 = 32767 (X);
>         rts;
>
> _stest_sat_min:
>         nop;
>         nop;
>         nop;
>         R0 = -32768 (X);
>         rts;
>
> This patch has been tested on x86_64-pc-linux-gnu with make bootstrap and
> make -k check with no new failures, and on a cross-compiler to bfin-elf
> with no regressions.  Ok for mainline?
>
>
> 2021-10-25  Roger Sayle  <roger@nextmovesoftware.com>
>
> gcc/ChangeLog
> 	* simplify-rtx (simplify_binary_operation_1) [SS_ASHIFT]: Simplify
> 	shifts of the mode's smin_value and smax_value when the bit count
> 	operand doesn't have side-effects.
> 	[US_ASHIFT]: Likewise, simplify shifts of the mode's umax_value
> 	when the bit count operand doesn't have side-effects.
> 	(simplify_const_binary_operation) [SS_ASHIFT, US_ASHIFT]: Perform
> 	compile-time evaluation of saturating left shifts with constant
> 	arguments.
>
> gcc/testsuite/ChangeLog
> 	* gcc.target/bfin/ssashift-1.c: New test case.
>
>
> Roger
> --
>
> diff --git a/gcc/simplify-rtx.c b/gcc/simplify-rtx.c
> index 2bb18fb..5903d55 100644
> --- a/gcc/simplify-rtx.c
> +++ b/gcc/simplify-rtx.c
> @@ -4064,9 +4064,25 @@ simplify_context::simplify_binary_operation_1 (rtx_code code,
>  	}
>        break;
>  
> -    case ASHIFT:
>      case SS_ASHIFT:
> +      if (CONST_INT_P (trueop0)
> +	  && HWI_COMPUTABLE_MODE_P (mode)
> +	  && (UINTVAL (trueop0) == (GET_MODE_MASK (mode) >> 1)
> +	      || mode_signbit_p (mode, trueop0))
> +	  && ! side_effects_p (op1))
> +	return op0;
> +      goto simplify_ashift;
> +
>      case US_ASHIFT:
> +      if (CONST_INT_P (trueop0)
> +	  && HWI_COMPUTABLE_MODE_P (mode)
> +	  && UINTVAL (trueop0) == GET_MODE_MASK (mode)
> +	  && ! side_effects_p (op1))
> +	return op0;
> +      /* FALLTHRU */
> +
> +    case ASHIFT:
> +simplify_ashift:
>        if (trueop1 == CONST0_RTX (mode))
>  	return op0;
>        if (trueop0 == CONST0_RTX (mode) && ! side_effects_p (op1))
> @@ -5004,6 +5020,8 @@ simplify_const_binary_operation (enum rtx_code code, machine_mode mode,
>  	case LSHIFTRT:
>  	case ASHIFTRT:
>  	case ASHIFT:
> +	case SS_ASHIFT:
> +	case US_ASHIFT:
>  	  {
>  	    wide_int wop1 = pop1;
>  	    if (SHIFT_COUNT_TRUNCATED)
> @@ -5025,6 +5043,27 @@ simplify_const_binary_operation (enum rtx_code code, machine_mode mode,
>  		result = wi::lshift (pop0, wop1);
>  		break;
>  
> +	      case SS_ASHIFT:
> +		if (wi::leu_p (wop1, wi::clrsb (pop0)))
> +		  result = wi::lshift (pop0, wop1);
> +		else if (wi::neg_p (pop0))
> +		  result = wi::min_value (GET_MODE_PRECISION (int_mode),
> +					  SIGNED);
> +		else
> +		  result = wi::max_value (GET_MODE_PRECISION (int_mode),
> +					  SIGNED);
> +		break;
> +
> +	      case US_ASHIFT:
> +		if (wi::eq_p (pop0, 0))
> +		  result = pop0;
> +		else if (wi::leu_p (wop1, wi::clz (pop0)))
> +		  result = wi::lshift (pop0, wop1);

I guess in the SS_ASHIFT case we're relying on clrsb (0) doing something
sensible (return the number of bits minus 1, which it does).  We could
also rely on wi::clz doing something sensible here (like we already do in
wi::min_precision) and remove the special case for zero.  Either way
is fine though.

> +		else
> +		  result = wi::max_value (GET_MODE_PRECISION (int_mode),
> +					  UNSIGNED);
> +		break;

Very minor clean-up, but: the modes can be passed directly to
wi::min_value and wi::max_value, due to overloads in rtl.h.

OK with that change (and with or without dropping the zero check).

Thanks,
Richard

> +
>  	      default:
>  		gcc_unreachable ();
>  	      }
> diff --git a/gcc/testsuite/gcc.target/bfin/ssashift-1.c b/gcc/testsuite/gcc.target/bfin/ssashift-1.c
> new file mode 100644
> index 0000000..aba90a6
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/bfin/ssashift-1.c
> @@ -0,0 +1,52 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2" } */
> +
> +int test_ok_pos()
> +{
> +  int x = 100;
> +  return __builtin_bfin_shl_fr1x32(x,24);
> +}
> +
> +int test_ok_neg()
> +{
> +  int x = -100;
> +  return __builtin_bfin_shl_fr1x32(x,24);
> +}
> +
> +int test_sat_max()
> +{
> +  int x = 10000;
> +  return __builtin_bfin_shl_fr1x32(x,24);
> +}
> +
> +int test_sat_min()
> +{
> +  int x = -10000;
> +  return __builtin_bfin_shl_fr1x32(x,24);
> +}
> +
> +short stest_ok_pos()
> +{
> +  short x = 100;
> +  return __builtin_bfin_shl_fr1x16(x,8);
> +}
> +
> +short stest_ok_neg()
> +{
> +  short x = -100;
> +  return __builtin_bfin_shl_fr1x16(x,8);
> +}
> +
> +short stest_sat_max()
> +{
> +  short x = 10000;
> +  return __builtin_bfin_shl_fr1x16(x,8);
> +}
> +
> +short stest_sat_min()
> +{
> +  short x = -10000;
> +  return __builtin_bfin_shl_fr1x16(x,8);
> +}
> +/* { dg-final { scan-assembler-not "\\(S\\)" } } */
> +/* { dg-final { scan-assembler-not "\\(V,S\\)" } } */
  

Patch

diff --git a/gcc/simplify-rtx.c b/gcc/simplify-rtx.c
index 2bb18fb..5903d55 100644
--- a/gcc/simplify-rtx.c
+++ b/gcc/simplify-rtx.c
@@ -4064,9 +4064,25 @@  simplify_context::simplify_binary_operation_1 (rtx_code code,
 	}
       break;
 
-    case ASHIFT:
     case SS_ASHIFT:
+      if (CONST_INT_P (trueop0)
+	  && HWI_COMPUTABLE_MODE_P (mode)
+	  && (UINTVAL (trueop0) == (GET_MODE_MASK (mode) >> 1)
+	      || mode_signbit_p (mode, trueop0))
+	  && ! side_effects_p (op1))
+	return op0;
+      goto simplify_ashift;
+
     case US_ASHIFT:
+      if (CONST_INT_P (trueop0)
+	  && HWI_COMPUTABLE_MODE_P (mode)
+	  && UINTVAL (trueop0) == GET_MODE_MASK (mode)
+	  && ! side_effects_p (op1))
+	return op0;
+      /* FALLTHRU */
+
+    case ASHIFT:
+simplify_ashift:
       if (trueop1 == CONST0_RTX (mode))
 	return op0;
       if (trueop0 == CONST0_RTX (mode) && ! side_effects_p (op1))
@@ -5004,6 +5020,8 @@  simplify_const_binary_operation (enum rtx_code code, machine_mode mode,
 	case LSHIFTRT:
 	case ASHIFTRT:
 	case ASHIFT:
+	case SS_ASHIFT:
+	case US_ASHIFT:
 	  {
 	    wide_int wop1 = pop1;
 	    if (SHIFT_COUNT_TRUNCATED)
@@ -5025,6 +5043,27 @@  simplify_const_binary_operation (enum rtx_code code, machine_mode mode,
 		result = wi::lshift (pop0, wop1);
 		break;
 
+	      case SS_ASHIFT:
+		if (wi::leu_p (wop1, wi::clrsb (pop0)))
+		  result = wi::lshift (pop0, wop1);
+		else if (wi::neg_p (pop0))
+		  result = wi::min_value (GET_MODE_PRECISION (int_mode),
+					  SIGNED);
+		else
+		  result = wi::max_value (GET_MODE_PRECISION (int_mode),
+					  SIGNED);
+		break;
+
+	      case US_ASHIFT:
+		if (wi::eq_p (pop0, 0))
+		  result = pop0;
+		else if (wi::leu_p (wop1, wi::clz (pop0)))
+		  result = wi::lshift (pop0, wop1);
+		else
+		  result = wi::max_value (GET_MODE_PRECISION (int_mode),
+					  UNSIGNED);
+		break;
+
 	      default:
 		gcc_unreachable ();
 	      }
diff --git a/gcc/testsuite/gcc.target/bfin/ssashift-1.c b/gcc/testsuite/gcc.target/bfin/ssashift-1.c
new file mode 100644
index 0000000..aba90a6
--- /dev/null
+++ b/gcc/testsuite/gcc.target/bfin/ssashift-1.c
@@ -0,0 +1,52 @@ 
+/* { dg-do compile } */
+/* { dg-options "-O2" } */
+
+int test_ok_pos()
+{
+  int x = 100;
+  return __builtin_bfin_shl_fr1x32(x,24);
+}
+
+int test_ok_neg()
+{
+  int x = -100;
+  return __builtin_bfin_shl_fr1x32(x,24);
+}
+
+int test_sat_max()
+{
+  int x = 10000;
+  return __builtin_bfin_shl_fr1x32(x,24);
+}
+
+int test_sat_min()
+{
+  int x = -10000;
+  return __builtin_bfin_shl_fr1x32(x,24);
+}
+
+short stest_ok_pos()
+{
+  short x = 100;
+  return __builtin_bfin_shl_fr1x16(x,8);
+}
+
+short stest_ok_neg()
+{
+  short x = -100;
+  return __builtin_bfin_shl_fr1x16(x,8);
+}
+
+short stest_sat_max()
+{
+  short x = 10000;
+  return __builtin_bfin_shl_fr1x16(x,8);
+}
+
+short stest_sat_min()
+{
+  short x = -10000;
+  return __builtin_bfin_shl_fr1x16(x,8);
+}
+/* { dg-final { scan-assembler-not "\\(S\\)" } } */
+/* { dg-final { scan-assembler-not "\\(V,S\\)" } } */