* [PATCH] c++: Avoid incorrect shortening of divisions [PR108365]
@ 2023-01-11 17:58 Jakub Jelinek
2023-01-12 19:37 ` Jason Merrill
0 siblings, 1 reply; 8+ messages in thread
From: Jakub Jelinek @ 2023-01-11 17:58 UTC (permalink / raw)
To: Jason Merrill; +Cc: gcc-patches
Hi!
The following testcase is miscompiled, because we shorten the division
in a case where it should not be shortened.
Divisions (and modulos) can be shortened if it is unsigned division/modulo,
or if it is signed division/modulo where we can prove the dividend will
not be the minimum signed value or divisor will not be -1, because e.g.
on sizeof(long long)==sizeof(int)*2 && __INT_MAX__ == 0x7fffffff targets
(-2147483647 - 1) / -1 is UB
but
(int) (-2147483648LL / -1LL) is not, it is -2147483648.
The primary aim of both the C and C++ FE division/modulo shortening I assume
was for the implicit integral promotions of {,signed,unsigned} {char,short}
and because at this point we have no VRP information etc., the shortening
is done if the integral promotion is from unsigned type for the divisor
or if the dividend is an integer constant other than -1.
This works fine for char/short -> int promotions when char/short have
smaller precision than int - unsigned char -> int or unsigned short -> int
will always be a positive int, so never the most negative.
Now, the C FE checks whether orig_op0 is TYPE_UNSIGNED where op0 is either
the same as orig_op0 or that promoted to int, I think that works fine,
if it isn't promoted, either the division/modulo common type will have the
same precision as op0 but then the division/modulo is unsigned and so
without UB, or it will be done in wider precision (e.g. because op1 has
wider precision), but then op0 can't be minimum signed value. Or it has
been promoted to int, but in that case it was again from narrower type and
so never minimum signed int.
But the C++ FE was checking if op0 is a NOP_EXPR from TYPE_UNSIGNED.
First of all, not sure if the operand of NOP_EXPR couldn't be non-integral
type where TYPE_UNSIGNED wouldn't be meaningful, but more importantly,
even if it is a cast from unsigned integral type, we only know it can't be
minimum signed value if it is a widening cast, if it is same precision or
narrowing cast, we know nothing.
So, the following patch for the NOP_EXPR cases checks just in case that
it is from integral type and more importantly checks it is a widening
conversion, and then next to it also allows op0 to be just unsigned,
promoted or not, as that is what the C FE will do for those cases too
and I believe it must work - either the division/modulo common type
will be that unsigned type, then we can shorten and don't need to worry
about UB, or it will be some wider signed type but then it can't be most
negative value of the wider type.
Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
2023-01-11 Jakub Jelinek <jakub@redhat.com>
PR c++/108365
* typeck.cc (cp_build_binary_op): For integral division or modulo,
shorten if type0 is unsigned, or op0 is cast from narrower unsigned
integral type or stripped_op1 is INTEGER_CST other than -1.
* g++.dg/opt/pr108365.C: New test.
* g++.dg/warn/pr108365.C: New test.
--- gcc/cp/typeck.cc.jj 2022-12-15 19:17:37.828072458 +0100
+++ gcc/cp/typeck.cc 2023-01-11 12:15:25.195284107 +0100
@@ -5455,8 +5455,15 @@ cp_build_binary_op (const op_location_t
point, so we have to dig out the original type to find out if
it was unsigned. */
tree stripped_op1 = tree_strip_any_location_wrapper (op1);
- shorten = ((TREE_CODE (op0) == NOP_EXPR
- && TYPE_UNSIGNED (TREE_TYPE (TREE_OPERAND (op0, 0))))
+ shorten = (TYPE_UNSIGNED (type0)
+ || (TREE_CODE (op0) == NOP_EXPR
+ && INTEGRAL_TYPE_P (TREE_TYPE (TREE_OPERAND (op0,
+ 0)))
+ && TYPE_UNSIGNED (TREE_TYPE (TREE_OPERAND (op0,
+ 0)))
+ && (TYPE_PRECISION (TREE_TYPE (TREE_OPERAND (op0,
+ 0)))
+ < TYPE_PRECISION (type0)))
|| (TREE_CODE (stripped_op1) == INTEGER_CST
&& ! integer_all_onesp (stripped_op1)));
}
@@ -5491,8 +5498,12 @@ cp_build_binary_op (const op_location_t
quotient can't be represented in the computation mode. We shorten
only if unsigned or if dividing by something we know != -1. */
tree stripped_op1 = tree_strip_any_location_wrapper (op1);
- shorten = ((TREE_CODE (op0) == NOP_EXPR
- && TYPE_UNSIGNED (TREE_TYPE (TREE_OPERAND (op0, 0))))
+ shorten = (TYPE_UNSIGNED (type0)
+ || (TREE_CODE (op0) == NOP_EXPR
+ && INTEGRAL_TYPE_P (TREE_TYPE (TREE_OPERAND (op0, 0)))
+ && TYPE_UNSIGNED (TREE_TYPE (TREE_OPERAND (op0, 0)))
+ && (TYPE_PRECISION (TREE_TYPE (TREE_OPERAND (op0, 0)))
+ < TYPE_PRECISION (type0)))
|| (TREE_CODE (stripped_op1) == INTEGER_CST
&& ! integer_all_onesp (stripped_op1)));
common = 1;
--- gcc/testsuite/g++.dg/opt/pr108365.C.jj 2023-01-11 12:19:03.322086288 +0100
+++ gcc/testsuite/g++.dg/opt/pr108365.C 2023-01-11 12:18:39.811430975 +0100
@@ -0,0 +1,13 @@
+// PR c++/108365
+// { dg-do run }
+
+char b = 1;
+
+int
+main ()
+{
+#if __CHAR_BIT__ == 8 && __SIZEOF_SHORT__ == 2 && __SIZEOF_INT__ == 4 && __SIZEOF_LONG_LONG__ == 8
+ while ((short) ((long long) (unsigned long long) (-__INT_MAX__ - 1) / (long long) (b ? -1 : 0)))
+ ;
+#endif
+}
--- gcc/testsuite/g++.dg/warn/pr108365.C.jj 2023-01-11 12:32:55.952875172 +0100
+++ gcc/testsuite/g++.dg/warn/pr108365.C 2023-01-11 12:32:37.345148131 +0100
@@ -0,0 +1,5 @@
+// PR c++/108365
+// { dg-do compile { target { { { ilp32 || lp64 } || llp64 } && c++11 } } }
+
+constexpr char b = 1;
+long t = (short) ((long long) (unsigned long long) (-__INT_MAX__ - 1) / (long long) (b ? -1 : 0)); // { dg-bogus "integer overflow in expression of type" }
Jakub
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] c++: Avoid incorrect shortening of divisions [PR108365]
2023-01-11 17:58 [PATCH] c++: Avoid incorrect shortening of divisions [PR108365] Jakub Jelinek
@ 2023-01-12 19:37 ` Jason Merrill
2023-01-12 19:55 ` Jakub Jelinek
0 siblings, 1 reply; 8+ messages in thread
From: Jason Merrill @ 2023-01-12 19:37 UTC (permalink / raw)
To: Jakub Jelinek; +Cc: gcc-patches
On 1/11/23 12:58, Jakub Jelinek wrote:
> Hi!
>
> The following testcase is miscompiled, because we shorten the division
> in a case where it should not be shortened.
> Divisions (and modulos) can be shortened if it is unsigned division/modulo,
> or if it is signed division/modulo where we can prove the dividend will
> not be the minimum signed value or divisor will not be -1, because e.g.
> on sizeof(long long)==sizeof(int)*2 && __INT_MAX__ == 0x7fffffff targets
> (-2147483647 - 1) / -1 is UB
> but
> (int) (-2147483648LL / -1LL) is not, it is -2147483648.
> The primary aim of both the C and C++ FE division/modulo shortening I assume
> was for the implicit integral promotions of {,signed,unsigned} {char,short}
> and because at this point we have no VRP information etc., the shortening
> is done if the integral promotion is from unsigned type for the divisor
> or if the dividend is an integer constant other than -1.
> This works fine for char/short -> int promotions when char/short have
> smaller precision than int - unsigned char -> int or unsigned short -> int
> will always be a positive int, so never the most negative.
>
> Now, the C FE checks whether orig_op0 is TYPE_UNSIGNED where op0 is either
> the same as orig_op0 or that promoted to int, I think that works fine,
> if it isn't promoted, either the division/modulo common type will have the
> same precision as op0 but then the division/modulo is unsigned and so
> without UB, or it will be done in wider precision (e.g. because op1 has
> wider precision), but then op0 can't be minimum signed value. Or it has
> been promoted to int, but in that case it was again from narrower type and
> so never minimum signed int.
>
> But the C++ FE was checking if op0 is a NOP_EXPR from TYPE_UNSIGNED.
> First of all, not sure if the operand of NOP_EXPR couldn't be non-integral
> type where TYPE_UNSIGNED wouldn't be meaningful, but more importantly,
> even if it is a cast from unsigned integral type, we only know it can't be
> minimum signed value if it is a widening cast, if it is same precision or
> narrowing cast, we know nothing.
Curious, this divergence goes back to 1994, when the C++ front-end was
merged and tege changed the condition in the C front-end.
> So, the following patch for the NOP_EXPR cases checks just in case that
> it is from integral type and more importantly checks it is a widening
> conversion, and then next to it also allows op0 to be just unsigned,
> promoted or not, as that is what the C FE will do for those cases too
> and I believe it must work - either the division/modulo common type
> will be that unsigned type, then we can shorten and don't need to worry
> about UB, or it will be some wider signed type but then it can't be most
> negative value of the wider type.
Why not use the same condition in C and C++?
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
>
> 2023-01-11 Jakub Jelinek <jakub@redhat.com>
>
> PR c++/108365
> * typeck.cc (cp_build_binary_op): For integral division or modulo,
> shorten if type0 is unsigned, or op0 is cast from narrower unsigned
> integral type or stripped_op1 is INTEGER_CST other than -1.
>
> * g++.dg/opt/pr108365.C: New test.
> * g++.dg/warn/pr108365.C: New test.
>
> --- gcc/cp/typeck.cc.jj 2022-12-15 19:17:37.828072458 +0100
> +++ gcc/cp/typeck.cc 2023-01-11 12:15:25.195284107 +0100
> @@ -5455,8 +5455,15 @@ cp_build_binary_op (const op_location_t
> point, so we have to dig out the original type to find out if
> it was unsigned. */
> tree stripped_op1 = tree_strip_any_location_wrapper (op1);
> - shorten = ((TREE_CODE (op0) == NOP_EXPR
> - && TYPE_UNSIGNED (TREE_TYPE (TREE_OPERAND (op0, 0))))
> + shorten = (TYPE_UNSIGNED (type0)
> + || (TREE_CODE (op0) == NOP_EXPR
> + && INTEGRAL_TYPE_P (TREE_TYPE (TREE_OPERAND (op0,
> + 0)))
> + && TYPE_UNSIGNED (TREE_TYPE (TREE_OPERAND (op0,
> + 0)))
> + && (TYPE_PRECISION (TREE_TYPE (TREE_OPERAND (op0,
> + 0)))
> + < TYPE_PRECISION (type0)))
> || (TREE_CODE (stripped_op1) == INTEGER_CST
> && ! integer_all_onesp (stripped_op1)));
> }
> @@ -5491,8 +5498,12 @@ cp_build_binary_op (const op_location_t
> quotient can't be represented in the computation mode. We shorten
> only if unsigned or if dividing by something we know != -1. */
> tree stripped_op1 = tree_strip_any_location_wrapper (op1);
> - shorten = ((TREE_CODE (op0) == NOP_EXPR
> - && TYPE_UNSIGNED (TREE_TYPE (TREE_OPERAND (op0, 0))))
> + shorten = (TYPE_UNSIGNED (type0)
> + || (TREE_CODE (op0) == NOP_EXPR
> + && INTEGRAL_TYPE_P (TREE_TYPE (TREE_OPERAND (op0, 0)))
> + && TYPE_UNSIGNED (TREE_TYPE (TREE_OPERAND (op0, 0)))
> + && (TYPE_PRECISION (TREE_TYPE (TREE_OPERAND (op0, 0)))
> + < TYPE_PRECISION (type0)))
> || (TREE_CODE (stripped_op1) == INTEGER_CST
> && ! integer_all_onesp (stripped_op1)));
> common = 1;
> --- gcc/testsuite/g++.dg/opt/pr108365.C.jj 2023-01-11 12:19:03.322086288 +0100
> +++ gcc/testsuite/g++.dg/opt/pr108365.C 2023-01-11 12:18:39.811430975 +0100
> @@ -0,0 +1,13 @@
> +// PR c++/108365
> +// { dg-do run }
> +
> +char b = 1;
> +
> +int
> +main ()
> +{
> +#if __CHAR_BIT__ == 8 && __SIZEOF_SHORT__ == 2 && __SIZEOF_INT__ == 4 && __SIZEOF_LONG_LONG__ == 8
> + while ((short) ((long long) (unsigned long long) (-__INT_MAX__ - 1) / (long long) (b ? -1 : 0)))
> + ;
> +#endif
> +}
> --- gcc/testsuite/g++.dg/warn/pr108365.C.jj 2023-01-11 12:32:55.952875172 +0100
> +++ gcc/testsuite/g++.dg/warn/pr108365.C 2023-01-11 12:32:37.345148131 +0100
> @@ -0,0 +1,5 @@
> +// PR c++/108365
> +// { dg-do compile { target { { { ilp32 || lp64 } || llp64 } && c++11 } } }
> +
> +constexpr char b = 1;
> +long t = (short) ((long long) (unsigned long long) (-__INT_MAX__ - 1) / (long long) (b ? -1 : 0)); // { dg-bogus "integer overflow in expression of type" }
>
> Jakub
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] c++: Avoid incorrect shortening of divisions [PR108365]
2023-01-12 19:37 ` Jason Merrill
@ 2023-01-12 19:55 ` Jakub Jelinek
2023-01-12 20:31 ` [PATCH] c, c++, v2: " Jakub Jelinek
0 siblings, 1 reply; 8+ messages in thread
From: Jakub Jelinek @ 2023-01-12 19:55 UTC (permalink / raw)
To: Jason Merrill; +Cc: gcc-patches
On Thu, Jan 12, 2023 at 02:37:13PM -0500, Jason Merrill wrote:
> > But the C++ FE was checking if op0 is a NOP_EXPR from TYPE_UNSIGNED.
> > First of all, not sure if the operand of NOP_EXPR couldn't be non-integral
> > type where TYPE_UNSIGNED wouldn't be meaningful, but more importantly,
> > even if it is a cast from unsigned integral type, we only know it can't be
> > minimum signed value if it is a widening cast, if it is same precision or
> > narrowing cast, we know nothing.
>
> Curious, this divergence goes back to 1994, when the C++ front-end was
> merged and tege changed the condition in the C front-end.
And it was changed to match the modulo condition adjusted by rms in 1993.
> > So, the following patch for the NOP_EXPR cases checks just in case that
> > it is from integral type and more importantly checks it is a widening
> > conversion, and then next to it also allows op0 to be just unsigned,
> > promoted or not, as that is what the C FE will do for those cases too
> > and I believe it must work - either the division/modulo common type
> > will be that unsigned type, then we can shorten and don't need to worry
> > about UB, or it will be some wider signed type but then it can't be most
> > negative value of the wider type.
>
> Why not use the same condition in C and C++?
I can test that. Do you mean change the C FE to match the patched C++
or change C++ FE to just test TYPE_UNSIGNED (orig_op0)?
I think both should work, though what I wrote perhaps can shorten in more
cases. Can try to construct testcases where it differs...
Jakub
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH] c, c++, v2: Avoid incorrect shortening of divisions [PR108365]
2023-01-12 19:55 ` Jakub Jelinek
@ 2023-01-12 20:31 ` Jakub Jelinek
2023-01-13 0:25 ` Jakub Jelinek
2023-01-13 16:58 ` Jason Merrill
0 siblings, 2 replies; 8+ messages in thread
From: Jakub Jelinek @ 2023-01-12 20:31 UTC (permalink / raw)
To: Jason Merrill, gcc-patches
On Thu, Jan 12, 2023 at 08:55:32PM +0100, Jakub Jelinek via Gcc-patches wrote:
> > > So, the following patch for the NOP_EXPR cases checks just in case that
> > > it is from integral type and more importantly checks it is a widening
> > > conversion, and then next to it also allows op0 to be just unsigned,
> > > promoted or not, as that is what the C FE will do for those cases too
> > > and I believe it must work - either the division/modulo common type
> > > will be that unsigned type, then we can shorten and don't need to worry
> > > about UB, or it will be some wider signed type but then it can't be most
> > > negative value of the wider type.
> >
> > Why not use the same condition in C and C++?
>
> I can test that. Do you mean change the C FE to match the patched C++
> or change C++ FE to just test TYPE_UNSIGNED (orig_op0)?
> I think both should work, though what I wrote perhaps can shorten in more
> cases. Can try to construct testcases where it differs...
E.g.
int f1 (int x, int y) { return (unsigned) x / y; }
unsigned short f2 (unsigned short x, unsigned short y) { return (unsigned) x / y; }
unsigned int f3 (unsigned int x, unsigned int y) { return (long long) x / y; }
C++ FE before and after the patch shortens the division in f2 and f3,
C FE shortens only in f2. So using the C FE condition would be a regression
for C++.
Therefore I'm going to test following patch:
2023-01-12 Jakub Jelinek <jakub@redhat.com>
PR c++/108365
* c-typeck.cc (build_binary_op): For integral division or modulo,
shorten if type0 is unsigned, or op0 is cast from narrower unsigned
integral type or op1 is INTEGER_CST other than -1.
* typeck.cc (cp_build_binary_op): For integral division or modulo,
shorten if type0 is unsigned, or op0 is cast from narrower unsigned
integral type or stripped_op1 is INTEGER_CST other than -1.
* c-c++-common/pr108365.c: New test.
* g++.dg/opt/pr108365.C: New test.
* g++.dg/warn/pr108365.C: New test.
--- gcc/c/c-typeck.cc.jj 2022-11-13 12:29:08.197504249 +0100
+++ gcc/c/c-typeck.cc 2023-01-12 21:06:53.310875131 +0100
@@ -12431,7 +12431,14 @@ build_binary_op (location_t location, en
undefined if the quotient can't be represented in the
computation mode. We shorten only if unsigned or if
dividing by something we know != -1. */
- shorten = (TYPE_UNSIGNED (TREE_TYPE (orig_op0))
+ shorten = (TYPE_UNSIGNED (type0)
+ || (TREE_CODE (op0) == NOP_EXPR
+ && INTEGRAL_TYPE_P (TREE_TYPE (TREE_OPERAND (op0,
+ 0)))
+ && TYPE_UNSIGNED (TREE_TYPE (TREE_OPERAND (op0, 0)))
+ && (TYPE_PRECISION (TREE_TYPE (TREE_OPERAND (op0,
+ 0)))
+ < TYPE_PRECISION (type0)))
|| (TREE_CODE (op1) == INTEGER_CST
&& !integer_all_onesp (op1)));
common = 1;
@@ -12467,7 +12474,12 @@ build_binary_op (location_t location, en
on some targets, since the modulo instruction is undefined if the
quotient can't be represented in the computation mode. We shorten
only if unsigned or if dividing by something we know != -1. */
- shorten = (TYPE_UNSIGNED (TREE_TYPE (orig_op0))
+ shorten = (TYPE_UNSIGNED (type0)
+ || (TREE_CODE (op0) == NOP_EXPR
+ && INTEGRAL_TYPE_P (TREE_TYPE (TREE_OPERAND (op0, 0)))
+ && TYPE_UNSIGNED (TREE_TYPE (TREE_OPERAND (op0, 0)))
+ && (TYPE_PRECISION (TREE_TYPE (TREE_OPERAND (op0, 0)))
+ < TYPE_PRECISION (type0)))
|| (TREE_CODE (op1) == INTEGER_CST
&& !integer_all_onesp (op1)));
common = 1;
--- gcc/cp/typeck.cc.jj 2023-01-11 12:47:56.099672340 +0100
+++ gcc/cp/typeck.cc 2023-01-12 21:04:23.738022528 +0100
@@ -5455,8 +5455,15 @@ cp_build_binary_op (const op_location_t
point, so we have to dig out the original type to find out if
it was unsigned. */
tree stripped_op1 = tree_strip_any_location_wrapper (op1);
- shorten = ((TREE_CODE (op0) == NOP_EXPR
- && TYPE_UNSIGNED (TREE_TYPE (TREE_OPERAND (op0, 0))))
+ shorten = (TYPE_UNSIGNED (type0)
+ || (TREE_CODE (op0) == NOP_EXPR
+ && INTEGRAL_TYPE_P (TREE_TYPE (TREE_OPERAND (op0,
+ 0)))
+ && TYPE_UNSIGNED (TREE_TYPE (TREE_OPERAND (op0,
+ 0)))
+ && (TYPE_PRECISION (TREE_TYPE (TREE_OPERAND (op0,
+ 0)))
+ < TYPE_PRECISION (type0)))
|| (TREE_CODE (stripped_op1) == INTEGER_CST
&& ! integer_all_onesp (stripped_op1)));
}
@@ -5491,8 +5498,12 @@ cp_build_binary_op (const op_location_t
quotient can't be represented in the computation mode. We shorten
only if unsigned or if dividing by something we know != -1. */
tree stripped_op1 = tree_strip_any_location_wrapper (op1);
- shorten = ((TREE_CODE (op0) == NOP_EXPR
- && TYPE_UNSIGNED (TREE_TYPE (TREE_OPERAND (op0, 0))))
+ shorten = (TYPE_UNSIGNED (type0)
+ || (TREE_CODE (op0) == NOP_EXPR
+ && INTEGRAL_TYPE_P (TREE_TYPE (TREE_OPERAND (op0, 0)))
+ && TYPE_UNSIGNED (TREE_TYPE (TREE_OPERAND (op0, 0)))
+ && (TYPE_PRECISION (TREE_TYPE (TREE_OPERAND (op0, 0)))
+ < TYPE_PRECISION (type0)))
|| (TREE_CODE (stripped_op1) == INTEGER_CST
&& ! integer_all_onesp (stripped_op1)));
common = 1;
--- gcc/testsuite/c-c++-common/pr108365.c.jj 2023-01-12 21:27:05.825467236 +0100
+++ gcc/testsuite/c-c++-common/pr108365.c 2023-01-12 21:26:44.095779209 +0100
@@ -0,0 +1,16 @@
+/* PR c++/108365 */
+/* { dg-do compile { target { { ilp32 || lp64 } || llp64 } } } */
+/* { dg-options "-O2 -fdump-tree-gimple" } */
+/* { dg-final { scan-tree-dump-not " \\\((int|unsigned short int|long long int|unsigned int)\\\) " "gimple" } } */
+
+unsigned short
+foo (unsigned short x, unsigned short y)
+{
+ return (unsigned) x / y;
+}
+
+unsigned int
+bar (unsigned int x, unsigned int y)
+{
+ return (long long) x / y;
+}
--- gcc/testsuite/g++.dg/opt/pr108365.C.jj 2023-01-12 21:04:23.738022528 +0100
+++ gcc/testsuite/g++.dg/opt/pr108365.C 2023-01-12 21:04:23.738022528 +0100
@@ -0,0 +1,13 @@
+// PR c++/108365
+// { dg-do run }
+
+char b = 1;
+
+int
+main ()
+{
+#if __CHAR_BIT__ == 8 && __SIZEOF_SHORT__ == 2 && __SIZEOF_INT__ == 4 && __SIZEOF_LONG_LONG__ == 8
+ while ((short) ((long long) (unsigned long long) (-__INT_MAX__ - 1) / (long long) (b ? -1 : 0)))
+ ;
+#endif
+}
--- gcc/testsuite/g++.dg/warn/pr108365.C.jj 2023-01-12 21:04:23.738022528 +0100
+++ gcc/testsuite/g++.dg/warn/pr108365.C 2023-01-12 21:04:23.738022528 +0100
@@ -0,0 +1,5 @@
+// PR c++/108365
+// { dg-do compile { target { { { ilp32 || lp64 } || llp64 } && c++11 } } }
+
+constexpr char b = 1;
+long t = (short) ((long long) (unsigned long long) (-__INT_MAX__ - 1) / (long long) (b ? -1 : 0)); // { dg-bogus "integer overflow in expression of type" }
Jakub
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] c, c++, v2: Avoid incorrect shortening of divisions [PR108365]
2023-01-12 20:31 ` [PATCH] c, c++, v2: " Jakub Jelinek
@ 2023-01-13 0:25 ` Jakub Jelinek
2023-01-13 16:58 ` Jason Merrill
1 sibling, 0 replies; 8+ messages in thread
From: Jakub Jelinek @ 2023-01-13 0:25 UTC (permalink / raw)
To: Jason Merrill, gcc-patches
On Thu, Jan 12, 2023 at 09:31:23PM +0100, Jakub Jelinek via Gcc-patches wrote:
> On Thu, Jan 12, 2023 at 08:55:32PM +0100, Jakub Jelinek via Gcc-patches wrote:
> > > > So, the following patch for the NOP_EXPR cases checks just in case that
> > > > it is from integral type and more importantly checks it is a widening
> > > > conversion, and then next to it also allows op0 to be just unsigned,
> > > > promoted or not, as that is what the C FE will do for those cases too
> > > > and I believe it must work - either the division/modulo common type
> > > > will be that unsigned type, then we can shorten and don't need to worry
> > > > about UB, or it will be some wider signed type but then it can't be most
> > > > negative value of the wider type.
> > >
> > > Why not use the same condition in C and C++?
> >
> > I can test that. Do you mean change the C FE to match the patched C++
> > or change C++ FE to just test TYPE_UNSIGNED (orig_op0)?
> > I think both should work, though what I wrote perhaps can shorten in more
> > cases. Can try to construct testcases where it differs...
>
> E.g.
> int f1 (int x, int y) { return (unsigned) x / y; }
> unsigned short f2 (unsigned short x, unsigned short y) { return (unsigned) x / y; }
> unsigned int f3 (unsigned int x, unsigned int y) { return (long long) x / y; }
> C++ FE before and after the patch shortens the division in f2 and f3,
> C FE shortens only in f2. So using the C FE condition would be a regression
> for C++.
>
> Therefore I'm going to test following patch:
Bootstrapped/regtested successfully on x86_64-linux and i686-linux.
Jakub
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] c, c++, v2: Avoid incorrect shortening of divisions [PR108365]
2023-01-12 20:31 ` [PATCH] c, c++, v2: " Jakub Jelinek
2023-01-13 0:25 ` Jakub Jelinek
@ 2023-01-13 16:58 ` Jason Merrill
2023-01-13 17:45 ` [PATCH] c, c++, v3: " Jakub Jelinek
1 sibling, 1 reply; 8+ messages in thread
From: Jason Merrill @ 2023-01-13 16:58 UTC (permalink / raw)
To: Jakub Jelinek, gcc-patches
On 1/12/23 15:31, Jakub Jelinek wrote:
> On Thu, Jan 12, 2023 at 08:55:32PM +0100, Jakub Jelinek via Gcc-patches wrote:
>>>> So, the following patch for the NOP_EXPR cases checks just in case that
>>>> it is from integral type and more importantly checks it is a widening
>>>> conversion, and then next to it also allows op0 to be just unsigned,
>>>> promoted or not, as that is what the C FE will do for those cases too
>>>> and I believe it must work - either the division/modulo common type
>>>> will be that unsigned type, then we can shorten and don't need to worry
>>>> about UB, or it will be some wider signed type but then it can't be most
>>>> negative value of the wider type.
>>>
>>> Why not use the same condition in C and C++?
>>
>> I can test that. Do you mean change the C FE to match the patched C++
>> or change C++ FE to just test TYPE_UNSIGNED (orig_op0)?
>> I think both should work, though what I wrote perhaps can shorten in more
>> cases. Can try to construct testcases where it differs...
>
> E.g.
> int f1 (int x, int y) { return (unsigned) x / y; }
> unsigned short f2 (unsigned short x, unsigned short y) { return (unsigned) x / y; }
> unsigned int f3 (unsigned int x, unsigned int y) { return (long long) x / y; }
> C++ FE before and after the patch shortens the division in f2 and f3,
> C FE shortens only in f2. So using the C FE condition would be a regression
> for C++.
>
> Therefore I'm going to test following patch:
LGTM, though we might put that condition in c-common somewhere?
> 2023-01-12 Jakub Jelinek <jakub@redhat.com>
>
> PR c++/108365
> * c-typeck.cc (build_binary_op): For integral division or modulo,
> shorten if type0 is unsigned, or op0 is cast from narrower unsigned
> integral type or op1 is INTEGER_CST other than -1.
>
> * typeck.cc (cp_build_binary_op): For integral division or modulo,
> shorten if type0 is unsigned, or op0 is cast from narrower unsigned
> integral type or stripped_op1 is INTEGER_CST other than -1.
>
> * c-c++-common/pr108365.c: New test.
> * g++.dg/opt/pr108365.C: New test.
> * g++.dg/warn/pr108365.C: New test.
>
> --- gcc/c/c-typeck.cc.jj 2022-11-13 12:29:08.197504249 +0100
> +++ gcc/c/c-typeck.cc 2023-01-12 21:06:53.310875131 +0100
> @@ -12431,7 +12431,14 @@ build_binary_op (location_t location, en
> undefined if the quotient can't be represented in the
> computation mode. We shorten only if unsigned or if
> dividing by something we know != -1. */
> - shorten = (TYPE_UNSIGNED (TREE_TYPE (orig_op0))
> + shorten = (TYPE_UNSIGNED (type0)
> + || (TREE_CODE (op0) == NOP_EXPR
> + && INTEGRAL_TYPE_P (TREE_TYPE (TREE_OPERAND (op0,
> + 0)))
> + && TYPE_UNSIGNED (TREE_TYPE (TREE_OPERAND (op0, 0)))
> + && (TYPE_PRECISION (TREE_TYPE (TREE_OPERAND (op0,
> + 0)))
> + < TYPE_PRECISION (type0)))
> || (TREE_CODE (op1) == INTEGER_CST
> && !integer_all_onesp (op1)));
> common = 1;
> @@ -12467,7 +12474,12 @@ build_binary_op (location_t location, en
> on some targets, since the modulo instruction is undefined if the
> quotient can't be represented in the computation mode. We shorten
> only if unsigned or if dividing by something we know != -1. */
> - shorten = (TYPE_UNSIGNED (TREE_TYPE (orig_op0))
> + shorten = (TYPE_UNSIGNED (type0)
> + || (TREE_CODE (op0) == NOP_EXPR
> + && INTEGRAL_TYPE_P (TREE_TYPE (TREE_OPERAND (op0, 0)))
> + && TYPE_UNSIGNED (TREE_TYPE (TREE_OPERAND (op0, 0)))
> + && (TYPE_PRECISION (TREE_TYPE (TREE_OPERAND (op0, 0)))
> + < TYPE_PRECISION (type0)))
> || (TREE_CODE (op1) == INTEGER_CST
> && !integer_all_onesp (op1)));
> common = 1;
> --- gcc/cp/typeck.cc.jj 2023-01-11 12:47:56.099672340 +0100
> +++ gcc/cp/typeck.cc 2023-01-12 21:04:23.738022528 +0100
> @@ -5455,8 +5455,15 @@ cp_build_binary_op (const op_location_t
> point, so we have to dig out the original type to find out if
> it was unsigned. */
> tree stripped_op1 = tree_strip_any_location_wrapper (op1);
> - shorten = ((TREE_CODE (op0) == NOP_EXPR
> - && TYPE_UNSIGNED (TREE_TYPE (TREE_OPERAND (op0, 0))))
> + shorten = (TYPE_UNSIGNED (type0)
> + || (TREE_CODE (op0) == NOP_EXPR
> + && INTEGRAL_TYPE_P (TREE_TYPE (TREE_OPERAND (op0,
> + 0)))
> + && TYPE_UNSIGNED (TREE_TYPE (TREE_OPERAND (op0,
> + 0)))
> + && (TYPE_PRECISION (TREE_TYPE (TREE_OPERAND (op0,
> + 0)))
> + < TYPE_PRECISION (type0)))
> || (TREE_CODE (stripped_op1) == INTEGER_CST
> && ! integer_all_onesp (stripped_op1)));
> }
> @@ -5491,8 +5498,12 @@ cp_build_binary_op (const op_location_t
> quotient can't be represented in the computation mode. We shorten
> only if unsigned or if dividing by something we know != -1. */
> tree stripped_op1 = tree_strip_any_location_wrapper (op1);
> - shorten = ((TREE_CODE (op0) == NOP_EXPR
> - && TYPE_UNSIGNED (TREE_TYPE (TREE_OPERAND (op0, 0))))
> + shorten = (TYPE_UNSIGNED (type0)
> + || (TREE_CODE (op0) == NOP_EXPR
> + && INTEGRAL_TYPE_P (TREE_TYPE (TREE_OPERAND (op0, 0)))
> + && TYPE_UNSIGNED (TREE_TYPE (TREE_OPERAND (op0, 0)))
> + && (TYPE_PRECISION (TREE_TYPE (TREE_OPERAND (op0, 0)))
> + < TYPE_PRECISION (type0)))
> || (TREE_CODE (stripped_op1) == INTEGER_CST
> && ! integer_all_onesp (stripped_op1)));
> common = 1;
> --- gcc/testsuite/c-c++-common/pr108365.c.jj 2023-01-12 21:27:05.825467236 +0100
> +++ gcc/testsuite/c-c++-common/pr108365.c 2023-01-12 21:26:44.095779209 +0100
> @@ -0,0 +1,16 @@
> +/* PR c++/108365 */
> +/* { dg-do compile { target { { ilp32 || lp64 } || llp64 } } } */
> +/* { dg-options "-O2 -fdump-tree-gimple" } */
> +/* { dg-final { scan-tree-dump-not " \\\((int|unsigned short int|long long int|unsigned int)\\\) " "gimple" } } */
> +
> +unsigned short
> +foo (unsigned short x, unsigned short y)
> +{
> + return (unsigned) x / y;
> +}
> +
> +unsigned int
> +bar (unsigned int x, unsigned int y)
> +{
> + return (long long) x / y;
> +}
> --- gcc/testsuite/g++.dg/opt/pr108365.C.jj 2023-01-12 21:04:23.738022528 +0100
> +++ gcc/testsuite/g++.dg/opt/pr108365.C 2023-01-12 21:04:23.738022528 +0100
> @@ -0,0 +1,13 @@
> +// PR c++/108365
> +// { dg-do run }
> +
> +char b = 1;
> +
> +int
> +main ()
> +{
> +#if __CHAR_BIT__ == 8 && __SIZEOF_SHORT__ == 2 && __SIZEOF_INT__ == 4 && __SIZEOF_LONG_LONG__ == 8
> + while ((short) ((long long) (unsigned long long) (-__INT_MAX__ - 1) / (long long) (b ? -1 : 0)))
> + ;
> +#endif
> +}
> --- gcc/testsuite/g++.dg/warn/pr108365.C.jj 2023-01-12 21:04:23.738022528 +0100
> +++ gcc/testsuite/g++.dg/warn/pr108365.C 2023-01-12 21:04:23.738022528 +0100
> @@ -0,0 +1,5 @@
> +// PR c++/108365
> +// { dg-do compile { target { { { ilp32 || lp64 } || llp64 } && c++11 } } }
> +
> +constexpr char b = 1;
> +long t = (short) ((long long) (unsigned long long) (-__INT_MAX__ - 1) / (long long) (b ? -1 : 0)); // { dg-bogus "integer overflow in expression of type" }
>
>
> Jakub
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH] c, c++, v3: Avoid incorrect shortening of divisions [PR108365]
2023-01-13 16:58 ` Jason Merrill
@ 2023-01-13 17:45 ` Jakub Jelinek
2023-01-13 19:08 ` Jason Merrill
0 siblings, 1 reply; 8+ messages in thread
From: Jakub Jelinek @ 2023-01-13 17:45 UTC (permalink / raw)
To: Jason Merrill; +Cc: gcc-patches
On Fri, Jan 13, 2023 at 11:58:06AM -0500, Jason Merrill wrote:
> LGTM, though we might put that condition in c-common somewhere?
So like this then? Just tested on the new testcases, full bootstrap/regtest
queued?
2023-01-13 Jakub Jelinek <jakub@redhat.com>
PR c++/108365
* c-common.h (may_shorten_divmod): New static inline function.
* c-typeck.cc (build_binary_op): Use may_shorten_divmod for integral
division or modulo.
* typeck.cc (cp_build_binary_op): Use may_shorten_divmod for integral
division or modulo.
* c-c++-common/pr108365.c: New test.
* g++.dg/opt/pr108365.C: New test.
* g++.dg/warn/pr108365.C: New test.
--- gcc/c-family/c-common.h.jj 2022-11-14 13:35:34.195160199 +0100
+++ gcc/c-family/c-common.h 2023-01-13 18:35:08.130362228 +0100
@@ -918,6 +918,30 @@ extern tree convert_init (tree, tree);
/* Subroutine of build_binary_op, used for certain operations. */
extern tree shorten_binary_op (tree result_type, tree op0, tree op1, bool bitwise);
+/* Return true if division or modulo op0 / op1 or op0 % op1 may be shortened.
+ We can shorten only if we can guarantee that op0 is not signed integral
+ minimum or op1 is not -1, because e.g. (long long) INT_MIN / -1 is
+ well defined INT_MAX + 1LL if long long is wider than int, but INT_MIN / -1
+ is UB. */
+static inline bool
+may_shorten_divmod (tree op0, tree op1)
+{
+ tree type0 = TREE_TYPE (op0);
+ if (TYPE_UNSIGNED (type0))
+ return true;
+ /* A cast from narrower unsigned won't be signed integral minimum,
+ but cast from same or wider precision unsigned could be. */
+ if (TREE_CODE (op0) == NOP_EXPR
+ && INTEGRAL_TYPE_P (TREE_TYPE (TREE_OPERAND (op0, 0)))
+ && TYPE_UNSIGNED (TREE_TYPE (TREE_OPERAND (op0, 0)))
+ && (TYPE_PRECISION (TREE_TYPE (TREE_OPERAND (op0, 0)))
+ < TYPE_PRECISION (type0)))
+ return true;
+ if (TREE_CODE (op1) == INTEGER_CST && !integer_all_onesp (op1))
+ return true;
+ return false;
+}
+
/* Subroutine of build_binary_op, used for comparison operations.
See if the operands have both been converted from subword integer types
and, if so, perhaps change them both back to their original type. */
--- gcc/c/c-typeck.cc.jj 2023-01-13 11:11:45.368016437 +0100
+++ gcc/c/c-typeck.cc 2023-01-13 18:38:25.919538847 +0100
@@ -12431,9 +12431,7 @@ build_binary_op (location_t location, en
undefined if the quotient can't be represented in the
computation mode. We shorten only if unsigned or if
dividing by something we know != -1. */
- shorten = (TYPE_UNSIGNED (TREE_TYPE (orig_op0))
- || (TREE_CODE (op1) == INTEGER_CST
- && !integer_all_onesp (op1)));
+ shorten = may_shorten_divmod (op0, op1);
common = 1;
}
break;
@@ -12467,9 +12465,7 @@ build_binary_op (location_t location, en
on some targets, since the modulo instruction is undefined if the
quotient can't be represented in the computation mode. We shorten
only if unsigned or if dividing by something we know != -1. */
- shorten = (TYPE_UNSIGNED (TREE_TYPE (orig_op0))
- || (TREE_CODE (op1) == INTEGER_CST
- && !integer_all_onesp (op1)));
+ shorten = may_shorten_divmod (op0, op1);
common = 1;
}
break;
--- gcc/cp/typeck.cc.jj 2023-01-13 11:11:45.418015716 +0100
+++ gcc/cp/typeck.cc 2023-01-13 18:38:40.754327078 +0100
@@ -5455,10 +5455,7 @@ cp_build_binary_op (const op_location_t
point, so we have to dig out the original type to find out if
it was unsigned. */
tree stripped_op1 = tree_strip_any_location_wrapper (op1);
- shorten = ((TREE_CODE (op0) == NOP_EXPR
- && TYPE_UNSIGNED (TREE_TYPE (TREE_OPERAND (op0, 0))))
- || (TREE_CODE (stripped_op1) == INTEGER_CST
- && ! integer_all_onesp (stripped_op1)));
+ shorten = may_shorten_divmod (op0, stripped_op1);
}
common = 1;
@@ -5491,10 +5488,7 @@ cp_build_binary_op (const op_location_t
quotient can't be represented in the computation mode. We shorten
only if unsigned or if dividing by something we know != -1. */
tree stripped_op1 = tree_strip_any_location_wrapper (op1);
- shorten = ((TREE_CODE (op0) == NOP_EXPR
- && TYPE_UNSIGNED (TREE_TYPE (TREE_OPERAND (op0, 0))))
- || (TREE_CODE (stripped_op1) == INTEGER_CST
- && ! integer_all_onesp (stripped_op1)));
+ shorten = may_shorten_divmod (op0, stripped_op1);
common = 1;
}
break;
--- gcc/testsuite/c-c++-common/pr108365.c.jj 2023-01-13 18:25:09.163911212 +0100
+++ gcc/testsuite/c-c++-common/pr108365.c 2023-01-13 18:25:09.163911212 +0100
@@ -0,0 +1,16 @@
+/* PR c++/108365 */
+/* { dg-do compile { target { { ilp32 || lp64 } || llp64 } } } */
+/* { dg-options "-O2 -fdump-tree-gimple" } */
+/* { dg-final { scan-tree-dump-not " \\\((int|unsigned short int|long long int|unsigned int)\\\) " "gimple" } } */
+
+unsigned short
+foo (unsigned short x, unsigned short y)
+{
+ return (unsigned) x / y;
+}
+
+unsigned int
+bar (unsigned int x, unsigned int y)
+{
+ return (long long) x / y;
+}
--- gcc/testsuite/g++.dg/opt/pr108365.C.jj 2023-01-13 18:25:09.163911212 +0100
+++ gcc/testsuite/g++.dg/opt/pr108365.C 2023-01-13 18:25:09.163911212 +0100
@@ -0,0 +1,13 @@
+// PR c++/108365
+// { dg-do run }
+
+char b = 1;
+
+int
+main ()
+{
+#if __CHAR_BIT__ == 8 && __SIZEOF_SHORT__ == 2 && __SIZEOF_INT__ == 4 && __SIZEOF_LONG_LONG__ == 8
+ while ((short) ((long long) (unsigned long long) (-__INT_MAX__ - 1) / (long long) (b ? -1 : 0)))
+ ;
+#endif
+}
--- gcc/testsuite/g++.dg/warn/pr108365.C.jj 2023-01-13 18:25:09.163911212 +0100
+++ gcc/testsuite/g++.dg/warn/pr108365.C 2023-01-13 18:25:09.163911212 +0100
@@ -0,0 +1,5 @@
+// PR c++/108365
+// { dg-do compile { target { { { ilp32 || lp64 } || llp64 } && c++11 } } }
+
+constexpr char b = 1;
+long t = (short) ((long long) (unsigned long long) (-__INT_MAX__ - 1) / (long long) (b ? -1 : 0)); // { dg-bogus "integer overflow in expression of type" }
Jakub
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] c, c++, v3: Avoid incorrect shortening of divisions [PR108365]
2023-01-13 17:45 ` [PATCH] c, c++, v3: " Jakub Jelinek
@ 2023-01-13 19:08 ` Jason Merrill
0 siblings, 0 replies; 8+ messages in thread
From: Jason Merrill @ 2023-01-13 19:08 UTC (permalink / raw)
To: Jakub Jelinek; +Cc: gcc-patches
On 1/13/23 12:45, Jakub Jelinek wrote:
> On Fri, Jan 13, 2023 at 11:58:06AM -0500, Jason Merrill wrote:
>> LGTM, though we might put that condition in c-common somewhere?
>
> So like this then? Just tested on the new testcases, full bootstrap/regtest
> queued?
OK..
> 2023-01-13 Jakub Jelinek <jakub@redhat.com>
>
> PR c++/108365
> * c-common.h (may_shorten_divmod): New static inline function.
>
> * c-typeck.cc (build_binary_op): Use may_shorten_divmod for integral
> division or modulo.
>
> * typeck.cc (cp_build_binary_op): Use may_shorten_divmod for integral
> division or modulo.
>
> * c-c++-common/pr108365.c: New test.
> * g++.dg/opt/pr108365.C: New test.
> * g++.dg/warn/pr108365.C: New test.
>
> --- gcc/c-family/c-common.h.jj 2022-11-14 13:35:34.195160199 +0100
> +++ gcc/c-family/c-common.h 2023-01-13 18:35:08.130362228 +0100
> @@ -918,6 +918,30 @@ extern tree convert_init (tree, tree);
> /* Subroutine of build_binary_op, used for certain operations. */
> extern tree shorten_binary_op (tree result_type, tree op0, tree op1, bool bitwise);
>
> +/* Return true if division or modulo op0 / op1 or op0 % op1 may be shortened.
> + We can shorten only if we can guarantee that op0 is not signed integral
> + minimum or op1 is not -1, because e.g. (long long) INT_MIN / -1 is
> + well defined INT_MAX + 1LL if long long is wider than int, but INT_MIN / -1
> + is UB. */
> +static inline bool
> +may_shorten_divmod (tree op0, tree op1)
> +{
> + tree type0 = TREE_TYPE (op0);
> + if (TYPE_UNSIGNED (type0))
> + return true;
> + /* A cast from narrower unsigned won't be signed integral minimum,
> + but cast from same or wider precision unsigned could be. */
> + if (TREE_CODE (op0) == NOP_EXPR
> + && INTEGRAL_TYPE_P (TREE_TYPE (TREE_OPERAND (op0, 0)))
> + && TYPE_UNSIGNED (TREE_TYPE (TREE_OPERAND (op0, 0)))
> + && (TYPE_PRECISION (TREE_TYPE (TREE_OPERAND (op0, 0)))
> + < TYPE_PRECISION (type0)))
> + return true;
> + if (TREE_CODE (op1) == INTEGER_CST && !integer_all_onesp (op1))
> + return true;
> + return false;
> +}
> +
> /* Subroutine of build_binary_op, used for comparison operations.
> See if the operands have both been converted from subword integer types
> and, if so, perhaps change them both back to their original type. */
> --- gcc/c/c-typeck.cc.jj 2023-01-13 11:11:45.368016437 +0100
> +++ gcc/c/c-typeck.cc 2023-01-13 18:38:25.919538847 +0100
> @@ -12431,9 +12431,7 @@ build_binary_op (location_t location, en
> undefined if the quotient can't be represented in the
> computation mode. We shorten only if unsigned or if
> dividing by something we know != -1. */
> - shorten = (TYPE_UNSIGNED (TREE_TYPE (orig_op0))
> - || (TREE_CODE (op1) == INTEGER_CST
> - && !integer_all_onesp (op1)));
> + shorten = may_shorten_divmod (op0, op1);
> common = 1;
> }
> break;
> @@ -12467,9 +12465,7 @@ build_binary_op (location_t location, en
> on some targets, since the modulo instruction is undefined if the
> quotient can't be represented in the computation mode. We shorten
> only if unsigned or if dividing by something we know != -1. */
> - shorten = (TYPE_UNSIGNED (TREE_TYPE (orig_op0))
> - || (TREE_CODE (op1) == INTEGER_CST
> - && !integer_all_onesp (op1)));
> + shorten = may_shorten_divmod (op0, op1);
> common = 1;
> }
> break;
> --- gcc/cp/typeck.cc.jj 2023-01-13 11:11:45.418015716 +0100
> +++ gcc/cp/typeck.cc 2023-01-13 18:38:40.754327078 +0100
> @@ -5455,10 +5455,7 @@ cp_build_binary_op (const op_location_t
> point, so we have to dig out the original type to find out if
> it was unsigned. */
> tree stripped_op1 = tree_strip_any_location_wrapper (op1);
> - shorten = ((TREE_CODE (op0) == NOP_EXPR
> - && TYPE_UNSIGNED (TREE_TYPE (TREE_OPERAND (op0, 0))))
> - || (TREE_CODE (stripped_op1) == INTEGER_CST
> - && ! integer_all_onesp (stripped_op1)));
> + shorten = may_shorten_divmod (op0, stripped_op1);
> }
>
> common = 1;
> @@ -5491,10 +5488,7 @@ cp_build_binary_op (const op_location_t
> quotient can't be represented in the computation mode. We shorten
> only if unsigned or if dividing by something we know != -1. */
> tree stripped_op1 = tree_strip_any_location_wrapper (op1);
> - shorten = ((TREE_CODE (op0) == NOP_EXPR
> - && TYPE_UNSIGNED (TREE_TYPE (TREE_OPERAND (op0, 0))))
> - || (TREE_CODE (stripped_op1) == INTEGER_CST
> - && ! integer_all_onesp (stripped_op1)));
> + shorten = may_shorten_divmod (op0, stripped_op1);
> common = 1;
> }
> break;
> --- gcc/testsuite/c-c++-common/pr108365.c.jj 2023-01-13 18:25:09.163911212 +0100
> +++ gcc/testsuite/c-c++-common/pr108365.c 2023-01-13 18:25:09.163911212 +0100
> @@ -0,0 +1,16 @@
> +/* PR c++/108365 */
> +/* { dg-do compile { target { { ilp32 || lp64 } || llp64 } } } */
> +/* { dg-options "-O2 -fdump-tree-gimple" } */
> +/* { dg-final { scan-tree-dump-not " \\\((int|unsigned short int|long long int|unsigned int)\\\) " "gimple" } } */
> +
> +unsigned short
> +foo (unsigned short x, unsigned short y)
> +{
> + return (unsigned) x / y;
> +}
> +
> +unsigned int
> +bar (unsigned int x, unsigned int y)
> +{
> + return (long long) x / y;
> +}
> --- gcc/testsuite/g++.dg/opt/pr108365.C.jj 2023-01-13 18:25:09.163911212 +0100
> +++ gcc/testsuite/g++.dg/opt/pr108365.C 2023-01-13 18:25:09.163911212 +0100
> @@ -0,0 +1,13 @@
> +// PR c++/108365
> +// { dg-do run }
> +
> +char b = 1;
> +
> +int
> +main ()
> +{
> +#if __CHAR_BIT__ == 8 && __SIZEOF_SHORT__ == 2 && __SIZEOF_INT__ == 4 && __SIZEOF_LONG_LONG__ == 8
> + while ((short) ((long long) (unsigned long long) (-__INT_MAX__ - 1) / (long long) (b ? -1 : 0)))
> + ;
> +#endif
> +}
> --- gcc/testsuite/g++.dg/warn/pr108365.C.jj 2023-01-13 18:25:09.163911212 +0100
> +++ gcc/testsuite/g++.dg/warn/pr108365.C 2023-01-13 18:25:09.163911212 +0100
> @@ -0,0 +1,5 @@
> +// PR c++/108365
> +// { dg-do compile { target { { { ilp32 || lp64 } || llp64 } && c++11 } } }
> +
> +constexpr char b = 1;
> +long t = (short) ((long long) (unsigned long long) (-__INT_MAX__ - 1) / (long long) (b ? -1 : 0)); // { dg-bogus "integer overflow in expression of type" }
>
>
> Jakub
>
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2023-01-13 19:08 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-01-11 17:58 [PATCH] c++: Avoid incorrect shortening of divisions [PR108365] Jakub Jelinek
2023-01-12 19:37 ` Jason Merrill
2023-01-12 19:55 ` Jakub Jelinek
2023-01-12 20:31 ` [PATCH] c, c++, v2: " Jakub Jelinek
2023-01-13 0:25 ` Jakub Jelinek
2023-01-13 16:58 ` Jason Merrill
2023-01-13 17:45 ` [PATCH] c, c++, v3: " Jakub Jelinek
2023-01-13 19:08 ` Jason Merrill
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).