* [PATCH] lower-bitint: Fix _BitInt .{ADD,SUB}_OVERFLOW lowering [PR112750]
@ 2023-12-01 7:29 Jakub Jelinek
2023-12-01 7:51 ` Richard Biener
0 siblings, 1 reply; 2+ messages in thread
From: Jakub Jelinek @ 2023-12-01 7:29 UTC (permalink / raw)
To: Richard Biener; +Cc: gcc-patches
Hi!
The .{ADD,SUB}_OVERFLOW lowering is implemented by performing normal
addition/subtraction (perhaps extended to even more bits than normally by
continuing with extended values of operands after running of normal bits)
and in addition to that trying to compute if certain sets of bits are either
all zero or all sign extensions of the least significant bit.
That code is in a lot of cases guarded just by a single condition (which
can be idx > startlimb, idx >= startlimb or idx == startlimb) or by
2 conditions - if (idx >= startlimb) { if (idx == startlimb) ... else ... }
Now, if_then_if_then_else when the second argument is NULL works just as
if_then and sets m_gsi to be within the initially empty then block and that is
where we emit code for constant tidx startlimb + (cmp_code == GT_EXPR).
But in the 2 conditions case, m_gsi is set to the initially empty else
block, so using EQ_EXPR for the condition was incorrect (and strangely
nothing in the testsuite caught that), because the code for extracting the
lowest set of bits (i.e. when tidx is startlimb) is then done when idx
is not startlimb rather than when it is.
The following patch fixes that.
Note, when developing the lowering, I was using gcov to make sure all code
is covered by the testsuite with minimum exceptions, so no idea how this
slipped out.
Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
2023-12-01 Jakub Jelinek <jakub@redhat.com>
PR middle-end/112750
* gimple-lower-bitint.cc (bitint_large_huge::lower_addsub_overflow):
Use NE_EXPR rather than EQ_EXPR for g2 if !single_comparison and
adjust probabilities.
* gcc.dg/bitint-41.c: Use -std=c23 rather than -std=c2x.
* gcc.dg/torture/bitint-43.c: Likewise.
* gcc.dg/torture/bitint-44.c: Likewise.
* gcc.dg/torture/bitint-45.c: New test.
--- gcc/gimple-lower-bitint.cc.jj 2023-11-24 11:30:24.416427246 +0100
+++ gcc/gimple-lower-bitint.cc 2023-11-30 10:57:40.095555940 +0100
@@ -4028,11 +4028,11 @@ bitint_large_huge::lower_addsub_overflow
edge edge_true_true, edge_true_false, edge_false;
gimple *g2 = NULL;
if (!single_comparison)
- g2 = gimple_build_cond (EQ_EXPR, idx,
+ g2 = gimple_build_cond (NE_EXPR, idx,
size_int (startlimb), NULL_TREE,
NULL_TREE);
if_then_if_then_else (g, g2, profile_probability::likely (),
- profile_probability::unlikely (),
+ profile_probability::likely (),
edge_true_true, edge_true_false,
edge_false);
unsigned tidx = startlimb + (cmp_code == GT_EXPR);
--- gcc/testsuite/gcc.dg/bitint-41.c.jj 2023-11-23 12:59:48.027443972 +0100
+++ gcc/testsuite/gcc.dg/bitint-41.c 2023-11-30 11:05:54.956550967 +0100
@@ -1,6 +1,6 @@
/* PR middle-end/112336 */
/* { dg-do compile { target bitint } } */
-/* { dg-options "-std=c2x" } */
+/* { dg-options "-std=c23" } */
unsigned _BitInt(1) v1;
unsigned _BitInt(1) *p1 = &v1;
--- gcc/testsuite/gcc.dg/torture/bitint-43.c.jj 2023-11-20 09:49:35.236668167 +0100
+++ gcc/testsuite/gcc.dg/torture/bitint-43.c 2023-11-30 11:06:31.840028866 +0100
@@ -1,6 +1,6 @@
/* PR c/111309 */
/* { dg-do run { target bitint } } */
-/* { dg-options "-std=c2x -pedantic-errors" } */
+/* { dg-options "-std=c23 -pedantic-errors" } */
/* { dg-skip-if "" { ! run_expensive_tests } { "*" } { "-O0" "-O2" } } */
/* { dg-skip-if "" { ! run_expensive_tests } { "-flto" } { "" } } */
--- gcc/testsuite/gcc.dg/torture/bitint-44.c.jj 2023-11-14 10:52:16.192276014 +0100
+++ gcc/testsuite/gcc.dg/torture/bitint-44.c 2023-11-30 11:06:37.494948817 +0100
@@ -1,6 +1,6 @@
/* PR c/111309 */
/* { dg-do run { target bitint } } */
-/* { dg-options "-std=c2x -pedantic-errors" } */
+/* { dg-options "-std=c23 -pedantic-errors" } */
/* { dg-skip-if "" { ! run_expensive_tests } { "*" } { "-O0" "-O2" } } */
/* { dg-skip-if "" { ! run_expensive_tests } { "-flto" } { "" } } */
--- gcc/testsuite/gcc.dg/torture/bitint-45.c.jj 2023-11-30 11:07:40.778053018 +0100
+++ gcc/testsuite/gcc.dg/torture/bitint-45.c 2023-11-30 11:07:19.294357123 +0100
@@ -0,0 +1,32 @@
+/* PR middle-end/112750 */
+/* { 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__ >= 256
+_BitInt(256) a = __INT_MAX__ + (_BitInt(256)) 1;
+_BitInt(256) b = __INT_MAX__;
+#endif
+#if __BITINT_MAXWIDTH__ >= 512
+_BitInt(512) c = 0x7fffffffffffffffffffffffffffffffffffffffwb + (_BitInt(512)) 1;
+_BitInt(512) d = 0x7fffffffffffffffffffffffffffffffffffffffwb;
+#endif
+
+int
+main ()
+{
+#if __BITINT_MAXWIDTH__ >= 256
+ if (!__builtin_sub_overflow_p (a, 0, 0))
+ __builtin_abort ();
+ if (!__builtin_sub_overflow_p (b, -1, 0))
+ __builtin_abort ();
+#endif
+#if __BITINT_MAXWIDTH__ >= 512
+ if (!__builtin_sub_overflow_p (c, 0, (_BitInt(160)) 0))
+ __builtin_abort ();
+ if (!__builtin_sub_overflow_p (d, -1, 0))
+ __builtin_abort ();
+#endif
+ return 0;
+}
Jakub
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [PATCH] lower-bitint: Fix _BitInt .{ADD,SUB}_OVERFLOW lowering [PR112750]
2023-12-01 7:29 [PATCH] lower-bitint: Fix _BitInt .{ADD,SUB}_OVERFLOW lowering [PR112750] Jakub Jelinek
@ 2023-12-01 7:51 ` Richard Biener
0 siblings, 0 replies; 2+ messages in thread
From: Richard Biener @ 2023-12-01 7:51 UTC (permalink / raw)
To: Jakub Jelinek; +Cc: gcc-patches
On Fri, 1 Dec 2023, Jakub Jelinek wrote:
> Hi!
>
> The .{ADD,SUB}_OVERFLOW lowering is implemented by performing normal
> addition/subtraction (perhaps extended to even more bits than normally by
> continuing with extended values of operands after running of normal bits)
> and in addition to that trying to compute if certain sets of bits are either
> all zero or all sign extensions of the least significant bit.
>
> That code is in a lot of cases guarded just by a single condition (which
> can be idx > startlimb, idx >= startlimb or idx == startlimb) or by
> 2 conditions - if (idx >= startlimb) { if (idx == startlimb) ... else ... }
> Now, if_then_if_then_else when the second argument is NULL works just as
> if_then and sets m_gsi to be within the initially empty then block and that is
> where we emit code for constant tidx startlimb + (cmp_code == GT_EXPR).
> But in the 2 conditions case, m_gsi is set to the initially empty else
> block, so using EQ_EXPR for the condition was incorrect (and strangely
> nothing in the testsuite caught that), because the code for extracting the
> lowest set of bits (i.e. when tidx is startlimb) is then done when idx
> is not startlimb rather than when it is.
> The following patch fixes that.
>
> Note, when developing the lowering, I was using gcov to make sure all code
> is covered by the testsuite with minimum exceptions, so no idea how this
> slipped out.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
OK.
Richard.
> 2023-12-01 Jakub Jelinek <jakub@redhat.com>
>
> PR middle-end/112750
> * gimple-lower-bitint.cc (bitint_large_huge::lower_addsub_overflow):
> Use NE_EXPR rather than EQ_EXPR for g2 if !single_comparison and
> adjust probabilities.
>
> * gcc.dg/bitint-41.c: Use -std=c23 rather than -std=c2x.
> * gcc.dg/torture/bitint-43.c: Likewise.
> * gcc.dg/torture/bitint-44.c: Likewise.
> * gcc.dg/torture/bitint-45.c: New test.
>
> --- gcc/gimple-lower-bitint.cc.jj 2023-11-24 11:30:24.416427246 +0100
> +++ gcc/gimple-lower-bitint.cc 2023-11-30 10:57:40.095555940 +0100
> @@ -4028,11 +4028,11 @@ bitint_large_huge::lower_addsub_overflow
> edge edge_true_true, edge_true_false, edge_false;
> gimple *g2 = NULL;
> if (!single_comparison)
> - g2 = gimple_build_cond (EQ_EXPR, idx,
> + g2 = gimple_build_cond (NE_EXPR, idx,
> size_int (startlimb), NULL_TREE,
> NULL_TREE);
> if_then_if_then_else (g, g2, profile_probability::likely (),
> - profile_probability::unlikely (),
> + profile_probability::likely (),
> edge_true_true, edge_true_false,
> edge_false);
> unsigned tidx = startlimb + (cmp_code == GT_EXPR);
> --- gcc/testsuite/gcc.dg/bitint-41.c.jj 2023-11-23 12:59:48.027443972 +0100
> +++ gcc/testsuite/gcc.dg/bitint-41.c 2023-11-30 11:05:54.956550967 +0100
> @@ -1,6 +1,6 @@
> /* PR middle-end/112336 */
> /* { dg-do compile { target bitint } } */
> -/* { dg-options "-std=c2x" } */
> +/* { dg-options "-std=c23" } */
>
> unsigned _BitInt(1) v1;
> unsigned _BitInt(1) *p1 = &v1;
> --- gcc/testsuite/gcc.dg/torture/bitint-43.c.jj 2023-11-20 09:49:35.236668167 +0100
> +++ gcc/testsuite/gcc.dg/torture/bitint-43.c 2023-11-30 11:06:31.840028866 +0100
> @@ -1,6 +1,6 @@
> /* PR c/111309 */
> /* { dg-do run { target bitint } } */
> -/* { dg-options "-std=c2x -pedantic-errors" } */
> +/* { dg-options "-std=c23 -pedantic-errors" } */
> /* { dg-skip-if "" { ! run_expensive_tests } { "*" } { "-O0" "-O2" } } */
> /* { dg-skip-if "" { ! run_expensive_tests } { "-flto" } { "" } } */
>
> --- gcc/testsuite/gcc.dg/torture/bitint-44.c.jj 2023-11-14 10:52:16.192276014 +0100
> +++ gcc/testsuite/gcc.dg/torture/bitint-44.c 2023-11-30 11:06:37.494948817 +0100
> @@ -1,6 +1,6 @@
> /* PR c/111309 */
> /* { dg-do run { target bitint } } */
> -/* { dg-options "-std=c2x -pedantic-errors" } */
> +/* { dg-options "-std=c23 -pedantic-errors" } */
> /* { dg-skip-if "" { ! run_expensive_tests } { "*" } { "-O0" "-O2" } } */
> /* { dg-skip-if "" { ! run_expensive_tests } { "-flto" } { "" } } */
>
> --- gcc/testsuite/gcc.dg/torture/bitint-45.c.jj 2023-11-30 11:07:40.778053018 +0100
> +++ gcc/testsuite/gcc.dg/torture/bitint-45.c 2023-11-30 11:07:19.294357123 +0100
> @@ -0,0 +1,32 @@
> +/* PR middle-end/112750 */
> +/* { 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__ >= 256
> +_BitInt(256) a = __INT_MAX__ + (_BitInt(256)) 1;
> +_BitInt(256) b = __INT_MAX__;
> +#endif
> +#if __BITINT_MAXWIDTH__ >= 512
> +_BitInt(512) c = 0x7fffffffffffffffffffffffffffffffffffffffwb + (_BitInt(512)) 1;
> +_BitInt(512) d = 0x7fffffffffffffffffffffffffffffffffffffffwb;
> +#endif
> +
> +int
> +main ()
> +{
> +#if __BITINT_MAXWIDTH__ >= 256
> + if (!__builtin_sub_overflow_p (a, 0, 0))
> + __builtin_abort ();
> + if (!__builtin_sub_overflow_p (b, -1, 0))
> + __builtin_abort ();
> +#endif
> +#if __BITINT_MAXWIDTH__ >= 512
> + if (!__builtin_sub_overflow_p (c, 0, (_BitInt(160)) 0))
> + __builtin_abort ();
> + if (!__builtin_sub_overflow_p (d, -1, 0))
> + __builtin_abort ();
> +#endif
> + return 0;
> +}
>
> Jakub
>
>
--
Richard Biener <rguenther@suse.de>
SUSE Software Solutions Germany GmbH,
Frankenstrasse 146, 90461 Nuernberg, Germany;
GF: Ivo Totev, Andrew McDonald, Werner Knoblich; (HRB 36809, AG Nuernberg)
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2023-12-01 7:54 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-12-01 7:29 [PATCH] lower-bitint: Fix _BitInt .{ADD,SUB}_OVERFLOW lowering [PR112750] Jakub Jelinek
2023-12-01 7:51 ` Richard Biener
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).