bitintlower: Fix up ?ROTATE_EXPR lowering [PR117847]

Message ID Z07CCkwhSOjGH1dM@tucnak
State New
Headers
Series bitintlower: Fix up ?ROTATE_EXPR lowering [PR117847] |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_gcc_build--master-arm success Build passed
linaro-tcwg-bot/tcwg_gcc_check--master-arm success Test passed
linaro-tcwg-bot/tcwg_gcc_build--master-aarch64 success Build passed
linaro-tcwg-bot/tcwg_gcc_check--master-aarch64 fail Patch failed to apply

Commit Message

Jakub Jelinek Dec. 3, 2024, 8:32 a.m. UTC
  Hi!

In the ?ROTATE_EXPR lowering I forgot to handle rotation by 0 correctly.
INTEGER_CST 0 is very unlikely, it would be probably folded away, but
a non-constant count can't use just p - n because then the shift count
is out of bounds for zero.

In the FE I use n == 0 ? x : (x << n) | (x >> (p - n)) but bitintlower
here isn't prepared at this point to have bb split and am not sure if
using COND_EXPR is a good idea either, so the patch uses (p - n) % p.
Perhaps I should just disable lowering the rotate in the FE for the
non-mode precision BITINT_TYPEs too.

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

2024-12-03  Jakub Jelinek  <jakub@redhat.com>

	PR middle-end/117847
	* gimple-lower-bitint.cc (gimple_lower_bitint) <case LROTATE_EXPR>:
	Use m = (p - n) % p instead of m = p - n for the other shift count.

	* gcc.dg/torture/bitint-75.c: New test.


	Jakub
  

Comments

Richard Biener Dec. 3, 2024, 9:31 a.m. UTC | #1
On Tue, 3 Dec 2024, Jakub Jelinek wrote:

> Hi!
> 
> In the ?ROTATE_EXPR lowering I forgot to handle rotation by 0 correctly.
> INTEGER_CST 0 is very unlikely, it would be probably folded away, but
> a non-constant count can't use just p - n because then the shift count
> is out of bounds for zero.
> 
> In the FE I use n == 0 ? x : (x << n) | (x >> (p - n)) but bitintlower
> here isn't prepared at this point to have bb split and am not sure if
> using COND_EXPR is a good idea either, so the patch uses (p - n) % p.
> Perhaps I should just disable lowering the rotate in the FE for the
> non-mode precision BITINT_TYPEs too.
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

OK.

Richard.

> 2024-12-03  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR middle-end/117847
> 	* gimple-lower-bitint.cc (gimple_lower_bitint) <case LROTATE_EXPR>:
> 	Use m = (p - n) % p instead of m = p - n for the other shift count.
> 
> 	* gcc.dg/torture/bitint-75.c: New test.
> 
> --- gcc/gimple-lower-bitint.cc.jj	2024-11-23 13:00:29.000000000 +0100
> +++ gcc/gimple-lower-bitint.cc	2024-12-03 00:50:58.839052842 +0100
> @@ -6233,11 +6233,20 @@ gimple_lower_bitint (void)
>  		  tree p = build_int_cst (TREE_TYPE (n),
>  					  TYPE_PRECISION (type));
>  		  if (TREE_CODE (n) == INTEGER_CST)
> -		    m = fold_build2 (MINUS_EXPR, TREE_TYPE (n), p, n);
> +		    {
> +		      if (integer_zerop (n))
> +			m = n;
> +		      else
> +			m = fold_build2 (MINUS_EXPR, TREE_TYPE (n), p, n);
> +		    }
>  		  else
>  		    {
> +		      tree tem = make_ssa_name (TREE_TYPE (n));
> +		      g = gimple_build_assign (tem, MINUS_EXPR, p, n);
> +		      gsi_insert_before (&gsi, g, GSI_SAME_STMT);
> +		      gimple_set_location (g, loc);
>  		      m = make_ssa_name (TREE_TYPE (n));
> -		      g = gimple_build_assign (m, MINUS_EXPR, p, n);
> +		      g = gimple_build_assign (m, TRUNC_MOD_EXPR, tem, p);
>  		      gsi_insert_before (&gsi, g, GSI_SAME_STMT);
>  		      gimple_set_location (g, loc);
>  		    }
> --- gcc/testsuite/gcc.dg/torture/bitint-75.c.jj	2024-12-02 11:51:52.448594565 +0100
> +++ gcc/testsuite/gcc.dg/torture/bitint-75.c	2024-12-02 11:51:43.387722691 +0100
> @@ -0,0 +1,27 @@
> +/* PR middle-end/117847 */
> +/* { dg-do run { target bitint } } */
> +/* { dg-options "-std=c23" } */
> +/* { dg-skip-if "" { ! run_expensive_tests }  { "*" } { "-O0" "-O2" } } */
> +/* { dg-skip-if "" { ! run_expensive_tests } { "-flto" } { "" } } */
> +
> +#if __BITINT_MAXWIDTH__ >= 512
> +typedef unsigned _BitInt(512) B;
> +
> +__attribute__((noipa)) B
> +foo (B a, int r)
> +{
> +  B b = __builtin_stdc_rotate_left (a, r);
> +  return b;
> +}
> +#endif
> +
> +int
> +main ()
> +{
> +#if __BITINT_MAXWIDTH__ >= 512
> +  B a = 0x4ad4fdecc8717d7c6f8b1afb82fdb742477ef2ab34057d1dcc79ba30c38a352dea3253c8c25126d98da02213ad54b90d2998f947941ea4b45e71c61dc1fe3192uwb;
> +  B x = foo (a, 0);
> +  if (x != a)
> +    __builtin_abort ();
> +#endif
> +}
> 
> 	Jakub
> 
>
  

Patch

--- gcc/gimple-lower-bitint.cc.jj	2024-11-23 13:00:29.000000000 +0100
+++ gcc/gimple-lower-bitint.cc	2024-12-03 00:50:58.839052842 +0100
@@ -6233,11 +6233,20 @@  gimple_lower_bitint (void)
 		  tree p = build_int_cst (TREE_TYPE (n),
 					  TYPE_PRECISION (type));
 		  if (TREE_CODE (n) == INTEGER_CST)
-		    m = fold_build2 (MINUS_EXPR, TREE_TYPE (n), p, n);
+		    {
+		      if (integer_zerop (n))
+			m = n;
+		      else
+			m = fold_build2 (MINUS_EXPR, TREE_TYPE (n), p, n);
+		    }
 		  else
 		    {
+		      tree tem = make_ssa_name (TREE_TYPE (n));
+		      g = gimple_build_assign (tem, MINUS_EXPR, p, n);
+		      gsi_insert_before (&gsi, g, GSI_SAME_STMT);
+		      gimple_set_location (g, loc);
 		      m = make_ssa_name (TREE_TYPE (n));
-		      g = gimple_build_assign (m, MINUS_EXPR, p, n);
+		      g = gimple_build_assign (m, TRUNC_MOD_EXPR, tem, p);
 		      gsi_insert_before (&gsi, g, GSI_SAME_STMT);
 		      gimple_set_location (g, loc);
 		    }
--- gcc/testsuite/gcc.dg/torture/bitint-75.c.jj	2024-12-02 11:51:52.448594565 +0100
+++ gcc/testsuite/gcc.dg/torture/bitint-75.c	2024-12-02 11:51:43.387722691 +0100
@@ -0,0 +1,27 @@ 
+/* PR middle-end/117847 */
+/* { dg-do run { target bitint } } */
+/* { dg-options "-std=c23" } */
+/* { dg-skip-if "" { ! run_expensive_tests }  { "*" } { "-O0" "-O2" } } */
+/* { dg-skip-if "" { ! run_expensive_tests } { "-flto" } { "" } } */
+
+#if __BITINT_MAXWIDTH__ >= 512
+typedef unsigned _BitInt(512) B;
+
+__attribute__((noipa)) B
+foo (B a, int r)
+{
+  B b = __builtin_stdc_rotate_left (a, r);
+  return b;
+}
+#endif
+
+int
+main ()
+{
+#if __BITINT_MAXWIDTH__ >= 512
+  B a = 0x4ad4fdecc8717d7c6f8b1afb82fdb742477ef2ab34057d1dcc79ba30c38a352dea3253c8c25126d98da02213ad54b90d2998f947941ea4b45e71c61dc1fe3192uwb;
+  B x = foo (a, 0);
+  if (x != a)
+    __builtin_abort ();
+#endif
+}