* [PATCH] fold-const: Fix up multiple_of_p [PR112733]
@ 2023-11-29 8:28 Jakub Jelinek
2023-11-29 9:13 ` Richard Sandiford
2023-11-29 9:13 ` Richard Biener
0 siblings, 2 replies; 3+ messages in thread
From: Jakub Jelinek @ 2023-11-29 8:28 UTC (permalink / raw)
To: Richard Biener, Richard Sandiford; +Cc: gcc-patches
Hi!
We ICE on the following testcase when wi::multiple_of_p is called on
widest_int 1 and -128 with UNSIGNED. I still need to work on the
actual wide-int.cc issue, the latest patch attached to the PR regressed
bitint-{38,39}.c, so will need to debug that, but there is a clear bug
on the fold-const.cc side as well - widest_int is a signed representation
by definition, using UNSIGNED with it certainly doesn't match what was
intended, because -128 as the second operand effectively means unsigned
131072 bit 0xfffff............ffff80 integer, not the signed char -128
that appeared in the source.
In the INTEGER_CST case a few lines above this we already use
case INTEGER_CST:
if (TREE_CODE (bottom) != INTEGER_CST || integer_zerop (bottom))
return false;
return wi::multiple_of_p (wi::to_widest (top), wi::to_widest (bottom),
SIGNED);
so I think using SIGNED with widest_int is best there (compared to the
other choices in the PR).
Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
2023-11-29 Jakub Jelinek <jakub@redhat.com>
PR middle-end/112733
* fold-const.cc (multiple_of_p): Pass SIGNED rather than
UNSIGNED for wi::multiple_of_p on widest_int arguments.
* gcc.dg/pr112733.c: New test.
--- gcc/fold-const.cc.jj 2023-11-28 08:46:28.345803059 +0100
+++ gcc/fold-const.cc 2023-11-28 17:16:26.872291024 +0100
@@ -14563,7 +14563,7 @@ multiple_of_p (tree type, const_tree top
&& TREE_CODE (op2) == INTEGER_CST
&& integer_pow2p (bottom)
&& wi::multiple_of_p (wi::to_widest (op2),
- wi::to_widest (bottom), UNSIGNED))
+ wi::to_widest (bottom), SIGNED))
return true;
op1 = gimple_assign_rhs1 (stmt);
--- gcc/testsuite/gcc.dg/pr112733.c.jj 2023-11-28 17:19:06.813048090 +0100
+++ gcc/testsuite/gcc.dg/pr112733.c 2023-11-28 17:18:45.331349335 +0100
@@ -0,0 +1,16 @@
+/* PR middle-end/112733 */
+/* { dg-do compile } */
+/* { dg-options "-O2" } */
+
+signed char a, c;
+short b;
+
+void
+foo (void)
+{
+ signed char *e = &a;
+ c = foo != 0;
+ *e &= c;
+ for (; b; --b)
+ *e &= -128;
+}
Jakub
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] fold-const: Fix up multiple_of_p [PR112733]
2023-11-29 8:28 [PATCH] fold-const: Fix up multiple_of_p [PR112733] Jakub Jelinek
@ 2023-11-29 9:13 ` Richard Sandiford
2023-11-29 9:13 ` Richard Biener
1 sibling, 0 replies; 3+ messages in thread
From: Richard Sandiford @ 2023-11-29 9:13 UTC (permalink / raw)
To: Jakub Jelinek; +Cc: Richard Biener, gcc-patches
Jakub Jelinek <jakub@redhat.com> writes:
> Hi!
>
> We ICE on the following testcase when wi::multiple_of_p is called on
> widest_int 1 and -128 with UNSIGNED. I still need to work on the
> actual wide-int.cc issue, the latest patch attached to the PR regressed
> bitint-{38,39}.c, so will need to debug that, but there is a clear bug
> on the fold-const.cc side as well - widest_int is a signed representation
> by definition, using UNSIGNED with it certainly doesn't match what was
> intended, because -128 as the second operand effectively means unsigned
> 131072 bit 0xfffff............ffff80 integer, not the signed char -128
> that appeared in the source.
>
> In the INTEGER_CST case a few lines above this we already use
> case INTEGER_CST:
> if (TREE_CODE (bottom) != INTEGER_CST || integer_zerop (bottom))
> return false;
> return wi::multiple_of_p (wi::to_widest (top), wi::to_widest (bottom),
> SIGNED);
> so I think using SIGNED with widest_int is best there (compared to the
> other choices in the PR).
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
>
> 2023-11-29 Jakub Jelinek <jakub@redhat.com>
>
> PR middle-end/112733
> * fold-const.cc (multiple_of_p): Pass SIGNED rather than
> UNSIGNED for wi::multiple_of_p on widest_int arguments.
>
> * gcc.dg/pr112733.c: New test.
OK, thanks.
Richard
> --- gcc/fold-const.cc.jj 2023-11-28 08:46:28.345803059 +0100
> +++ gcc/fold-const.cc 2023-11-28 17:16:26.872291024 +0100
> @@ -14563,7 +14563,7 @@ multiple_of_p (tree type, const_tree top
> && TREE_CODE (op2) == INTEGER_CST
> && integer_pow2p (bottom)
> && wi::multiple_of_p (wi::to_widest (op2),
> - wi::to_widest (bottom), UNSIGNED))
> + wi::to_widest (bottom), SIGNED))
> return true;
>
> op1 = gimple_assign_rhs1 (stmt);
> --- gcc/testsuite/gcc.dg/pr112733.c.jj 2023-11-28 17:19:06.813048090 +0100
> +++ gcc/testsuite/gcc.dg/pr112733.c 2023-11-28 17:18:45.331349335 +0100
> @@ -0,0 +1,16 @@
> +/* PR middle-end/112733 */
> +/* { dg-do compile } */
> +/* { dg-options "-O2" } */
> +
> +signed char a, c;
> +short b;
> +
> +void
> +foo (void)
> +{
> + signed char *e = &a;
> + c = foo != 0;
> + *e &= c;
> + for (; b; --b)
> + *e &= -128;
> +}
>
> Jakub
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] fold-const: Fix up multiple_of_p [PR112733]
2023-11-29 8:28 [PATCH] fold-const: Fix up multiple_of_p [PR112733] Jakub Jelinek
2023-11-29 9:13 ` Richard Sandiford
@ 2023-11-29 9:13 ` Richard Biener
1 sibling, 0 replies; 3+ messages in thread
From: Richard Biener @ 2023-11-29 9:13 UTC (permalink / raw)
To: Jakub Jelinek; +Cc: Richard Sandiford, gcc-patches
> Am 29.11.2023 um 09:29 schrieb Jakub Jelinek <jakub@redhat.com>:
>
> Hi!
>
> We ICE on the following testcase when wi::multiple_of_p is called on
> widest_int 1 and -128 with UNSIGNED. I still need to work on the
> actual wide-int.cc issue, the latest patch attached to the PR regressed
> bitint-{38,39}.c, so will need to debug that, but there is a clear bug
> on the fold-const.cc side as well - widest_int is a signed representation
> by definition, using UNSIGNED with it certainly doesn't match what was
> intended, because -128 as the second operand effectively means unsigned
> 131072 bit 0xfffff............ffff80 integer, not the signed char -128
> that appeared in the source.
>
> In the INTEGER_CST case a few lines above this we already use
> case INTEGER_CST:
> if (TREE_CODE (bottom) != INTEGER_CST || integer_zerop (bottom))
> return false;
> return wi::multiple_of_p (wi::to_widest (top), wi::to_widest (bottom),
> SIGNED);
> so I think using SIGNED with widest_int is best there (compared to the
> other choices in the PR).
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
Ok
Richard
> 2023-11-29 Jakub Jelinek <jakub@redhat.com>
>
> PR middle-end/112733
> * fold-const.cc (multiple_of_p): Pass SIGNED rather than
> UNSIGNED for wi::multiple_of_p on widest_int arguments.
>
> * gcc.dg/pr112733.c: New test.
>
> --- gcc/fold-const.cc.jj 2023-11-28 08:46:28.345803059 +0100
> +++ gcc/fold-const.cc 2023-11-28 17:16:26.872291024 +0100
> @@ -14563,7 +14563,7 @@ multiple_of_p (tree type, const_tree top
> && TREE_CODE (op2) == INTEGER_CST
> && integer_pow2p (bottom)
> && wi::multiple_of_p (wi::to_widest (op2),
> - wi::to_widest (bottom), UNSIGNED))
> + wi::to_widest (bottom), SIGNED))
> return true;
>
> op1 = gimple_assign_rhs1 (stmt);
> --- gcc/testsuite/gcc.dg/pr112733.c.jj 2023-11-28 17:19:06.813048090 +0100
> +++ gcc/testsuite/gcc.dg/pr112733.c 2023-11-28 17:18:45.331349335 +0100
> @@ -0,0 +1,16 @@
> +/* PR middle-end/112733 */
> +/* { dg-do compile } */
> +/* { dg-options "-O2" } */
> +
> +signed char a, c;
> +short b;
> +
> +void
> +foo (void)
> +{
> + signed char *e = &a;
> + c = foo != 0;
> + *e &= c;
> + for (; b; --b)
> + *e &= -128;
> +}
>
> Jakub
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2023-11-29 9:14 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-11-29 8:28 [PATCH] fold-const: Fix up multiple_of_p [PR112733] Jakub Jelinek
2023-11-29 9:13 ` Richard Sandiford
2023-11-29 9:13 ` 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).