lower-bitint: Fix up handling of unsigned INTEGER_CSTs operands with lots of 1s in the upper bits [PR113334]
Checks
Commit Message
Hi!
For INTEGER_CST operands, the code decides if it should emit the whole
INTEGER_CST into memory, or if there are enough upper bits either all 0s
or all 1s to warrant an optimization, where we use memory for lower limbs
or even just an INTEGER_CST for least significant limb and fill in the
rest of limbs with 0s or 1s. Unfortunately when not using
bitint_min_cst_precision, the code was using tree_int_cst_sgn (op) < 0
to determine whether to fill in the upper bits with 1s or 0s. That is
incorrect for TYPE_UNSIGNED INTEGER_CSTs which have higher limbs full of
ones, we really want to check here whether the most significant bit is
set or clear.
Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux, ok for
trunk?
2024-01-12 Jakub Jelinek <jakub@redhat.com>
PR tree-optimization/113334
* gimple-lower-bitint.cc (bitint_large_huge::handle_operand): Use
wi::neg_p (wi::to_wide (op)) instead of tree_int_cst_sgn (op) < 0
to determine if number should be extended by all ones rather than zero
extended.
* gcc.dg/torture/bitint-46.c: New test.
Jakub
Comments
On Fri, 12 Jan 2024, Jakub Jelinek wrote:
> Hi!
>
> For INTEGER_CST operands, the code decides if it should emit the whole
> INTEGER_CST into memory, or if there are enough upper bits either all 0s
> or all 1s to warrant an optimization, where we use memory for lower limbs
> or even just an INTEGER_CST for least significant limb and fill in the
> rest of limbs with 0s or 1s. Unfortunately when not using
> bitint_min_cst_precision, the code was using tree_int_cst_sgn (op) < 0
> to determine whether to fill in the upper bits with 1s or 0s. That is
> incorrect for TYPE_UNSIGNED INTEGER_CSTs which have higher limbs full of
> ones, we really want to check here whether the most significant bit is
> set or clear.
>
> Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux, ok for
> trunk?
OK.
> 2024-01-12 Jakub Jelinek <jakub@redhat.com>
>
> PR tree-optimization/113334
> * gimple-lower-bitint.cc (bitint_large_huge::handle_operand): Use
> wi::neg_p (wi::to_wide (op)) instead of tree_int_cst_sgn (op) < 0
> to determine if number should be extended by all ones rather than zero
> extended.
>
> * gcc.dg/torture/bitint-46.c: New test.
>
> --- gcc/gimple-lower-bitint.cc.jj 2024-01-11 14:27:26.000000000 +0100
> +++ gcc/gimple-lower-bitint.cc 2024-01-11 17:35:07.484557476 +0100
> @@ -997,7 +997,7 @@ bitint_large_huge::handle_operand (tree
> {
> unsigned int prec = TYPE_PRECISION (TREE_TYPE (op));
> unsigned rem = prec % (2 * limb_prec);
> - int ext = tree_int_cst_sgn (op) < 0 ? -1 : 0;
> + int ext = wi::neg_p (wi::to_wide (op)) ? -1 : 0;
> tree c = m_data[m_data_cnt];
> unsigned min_prec = TYPE_PRECISION (TREE_TYPE (c));
> g = gimple_build_cond (LT_EXPR, idx,
> --- gcc/testsuite/gcc.dg/torture/bitint-46.c.jj 2024-01-11 17:44:42.360409112 +0100
> +++ gcc/testsuite/gcc.dg/torture/bitint-46.c 2024-01-11 17:44:31.471564635 +0100
> @@ -0,0 +1,32 @@
> +/* PR tree-optimization/113334 */
> +/* { dg-do run { target bitint } } */
> +/* { dg-options "-std=c23 -pedantic-errors" } */
> +/* { dg-skip-if "" { ! run_expensive_tests } { "*" } { "-O0" "-O2" } } */
> +/* { dg-skip-if "" { ! run_expensive_tests } { "-flto" } { "" } } */
> +
> +#if __BITINT_MAXWIDTH__ >= 384
> +__attribute__((noipa)) _BitInt(384)
> +foo (int s)
> +{
> + _BitInt(384) z = (-(unsigned _BitInt(384)) 4) >> s;
> + return z;
> +}
> +#endif
> +
> +int
> +main ()
> +{
> +#if __BITINT_MAXWIDTH__ >= 384
> + if (foo (59) != 0x1fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffwb)
> + __builtin_abort ();
> + if (foo (0) != -4wb)
> + __builtin_abort ();
> + if (foo (1) != 0x7ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffewb)
> + __builtin_abort ();
> + if (foo (11) != 0x001fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffwb)
> + __builtin_abort ();
> + if (foo (123) != 0x1fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffwb)
> + __builtin_abort ();
> +#endif
> + return 0;
> +}
>
> Jakub
>
>
@@ -997,7 +997,7 @@ bitint_large_huge::handle_operand (tree
{
unsigned int prec = TYPE_PRECISION (TREE_TYPE (op));
unsigned rem = prec % (2 * limb_prec);
- int ext = tree_int_cst_sgn (op) < 0 ? -1 : 0;
+ int ext = wi::neg_p (wi::to_wide (op)) ? -1 : 0;
tree c = m_data[m_data_cnt];
unsigned min_prec = TYPE_PRECISION (TREE_TYPE (c));
g = gimple_build_cond (LT_EXPR, idx,
@@ -0,0 +1,32 @@
+/* PR tree-optimization/113334 */
+/* { dg-do run { target bitint } } */
+/* { dg-options "-std=c23 -pedantic-errors" } */
+/* { dg-skip-if "" { ! run_expensive_tests } { "*" } { "-O0" "-O2" } } */
+/* { dg-skip-if "" { ! run_expensive_tests } { "-flto" } { "" } } */
+
+#if __BITINT_MAXWIDTH__ >= 384
+__attribute__((noipa)) _BitInt(384)
+foo (int s)
+{
+ _BitInt(384) z = (-(unsigned _BitInt(384)) 4) >> s;
+ return z;
+}
+#endif
+
+int
+main ()
+{
+#if __BITINT_MAXWIDTH__ >= 384
+ if (foo (59) != 0x1fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffwb)
+ __builtin_abort ();
+ if (foo (0) != -4wb)
+ __builtin_abort ();
+ if (foo (1) != 0x7ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffewb)
+ __builtin_abort ();
+ if (foo (11) != 0x001fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffwb)
+ __builtin_abort ();
+ if (foo (123) != 0x1fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffwb)
+ __builtin_abort ();
+#endif
+ return 0;
+}