public inbox for gcc-bugs@sourceware.org
help / color / mirror / Atom feed
* [Bug tree-optimization/108365] New: [9/10/11/12/13 Regression] Wrong code with -O0
@ 2023-01-10 22:47 vsevolod.livinskiy at gmail dot com
2023-01-10 22:55 ` [Bug tree-optimization/108365] " pinskia at gcc dot gnu.org
` (15 more replies)
0 siblings, 16 replies; 17+ messages in thread
From: vsevolod.livinskiy at gmail dot com @ 2023-01-10 22:47 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108365
Bug ID: 108365
Summary: [9/10/11/12/13 Regression] Wrong code with -O0
Product: gcc
Version: 13.0
Status: UNCONFIRMED
Severity: normal
Priority: P3
Component: tree-optimization
Assignee: unassigned at gcc dot gnu.org
Reporter: vsevolod.livinskiy at gmail dot com
Target Milestone: ---
Link to the Compiler Explorer: https://godbolt.org/z/rMaofso7E
Reproducer:
char b[23] = {1};
int main() {
for (;short(long((unsigned long)(-2147483647 - 1)) / long(b[0] ? -1 :
0)););
}
Error:
>$ g++ -O0 repr.cpp && ./a.out
Floating point exception (core dumped)
gcc version 13.0.0 20230110 (e9a39ad7936815980013605b052b12425d56ead8)
^ permalink raw reply [flat|nested] 17+ messages in thread
* [Bug tree-optimization/108365] [9/10/11/12/13 Regression] Wrong code with -O0
2023-01-10 22:47 [Bug tree-optimization/108365] New: [9/10/11/12/13 Regression] Wrong code with -O0 vsevolod.livinskiy at gmail dot com
@ 2023-01-10 22:55 ` pinskia at gcc dot gnu.org
2023-01-10 22:55 ` pinskia at gcc dot gnu.org
` (14 subsequent siblings)
15 siblings, 0 replies; 17+ messages in thread
From: pinskia at gcc dot gnu.org @ 2023-01-10 22:55 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108365
Andrew Pinski <pinskia at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Last reconfirmed| |2023-01-10
Known to fail| |7.1.0, 8.1.0
Keywords| |diagnostic
Known to work| |6.1.0, 6.4.0
Status|UNCONFIRMED |NEW
Ever confirmed|0 |1
--- Comment #1 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
A compile time testcase:
constexpr char b[23] = {1};
long t = long((unsigned long)(-2147483647 - 1))
/ long(b[0] ? -1 : 0);
This should not warn with -std=c++11 .
Confirmed.
^ permalink raw reply [flat|nested] 17+ messages in thread
* [Bug tree-optimization/108365] [9/10/11/12/13 Regression] Wrong code with -O0
2023-01-10 22:47 [Bug tree-optimization/108365] New: [9/10/11/12/13 Regression] Wrong code with -O0 vsevolod.livinskiy at gmail dot com
2023-01-10 22:55 ` [Bug tree-optimization/108365] " pinskia at gcc dot gnu.org
@ 2023-01-10 22:55 ` pinskia at gcc dot gnu.org
2023-01-10 23:07 ` [Bug middle-end/108365] " jakub at gcc dot gnu.org
` (13 subsequent siblings)
15 siblings, 0 replies; 17+ messages in thread
From: pinskia at gcc dot gnu.org @ 2023-01-10 22:55 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108365
Andrew Pinski <pinskia at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Target Milestone|--- |10.5
^ permalink raw reply [flat|nested] 17+ messages in thread
* [Bug middle-end/108365] [9/10/11/12/13 Regression] Wrong code with -O0
2023-01-10 22:47 [Bug tree-optimization/108365] New: [9/10/11/12/13 Regression] Wrong code with -O0 vsevolod.livinskiy at gmail dot com
2023-01-10 22:55 ` [Bug tree-optimization/108365] " pinskia at gcc dot gnu.org
2023-01-10 22:55 ` pinskia at gcc dot gnu.org
@ 2023-01-10 23:07 ` jakub at gcc dot gnu.org
2023-01-10 23:15 ` pinskia at gcc dot gnu.org
` (12 subsequent siblings)
15 siblings, 0 replies; 17+ messages in thread
From: jakub at gcc dot gnu.org @ 2023-01-10 23:07 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108365
Jakub Jelinek <jakub at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
CC| |jakub at gcc dot gnu.org
--- Comment #2 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
It is UB on ilp32, but for lp64 it should be well defined.
Started with r9-1730-g9e392989053729d4d50
Slightly adjusted so that it is valid even on ilp32:
char b = 1;
int
main ()
{
while ((short) ((long long) (unsigned long long) (-__INT_MAX__ - 1) / (long
long) (b ? -1 : 0)))
;
}
^ permalink raw reply [flat|nested] 17+ messages in thread
* [Bug middle-end/108365] [9/10/11/12/13 Regression] Wrong code with -O0
2023-01-10 22:47 [Bug tree-optimization/108365] New: [9/10/11/12/13 Regression] Wrong code with -O0 vsevolod.livinskiy at gmail dot com
` (2 preceding siblings ...)
2023-01-10 23:07 ` [Bug middle-end/108365] " jakub at gcc dot gnu.org
@ 2023-01-10 23:15 ` pinskia at gcc dot gnu.org
2023-01-10 23:36 ` jakub at gcc dot gnu.org
` (11 subsequent siblings)
15 siblings, 0 replies; 17+ messages in thread
From: pinskia at gcc dot gnu.org @ 2023-01-10 23:15 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108365
Andrew Pinski <pinskia at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Known to work|6.1.0, 6.4.0 |
Known to fail| |6.1.0, 6.4.0
--- Comment #3 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
(In reply to Jakub Jelinek from comment #2)
> It is UB on ilp32, but for lp64 it should be well defined.
> Started with r9-1730-g9e392989053729d4d50
Then there is an older bug.
For the following C++ code:
constexpr char b = 1;
long t = (short) ((long long) (unsigned long long) (-__INT_MAX__ - 1) / (long
long) (b ? -1 : 0));
Should not produce any warnings but does, all the way back to GCC 6.1.0.
^ permalink raw reply [flat|nested] 17+ messages in thread
* [Bug middle-end/108365] [9/10/11/12/13 Regression] Wrong code with -O0
2023-01-10 22:47 [Bug tree-optimization/108365] New: [9/10/11/12/13 Regression] Wrong code with -O0 vsevolod.livinskiy at gmail dot com
` (3 preceding siblings ...)
2023-01-10 23:15 ` pinskia at gcc dot gnu.org
@ 2023-01-10 23:36 ` jakub at gcc dot gnu.org
2023-01-10 23:59 ` jakub at gcc dot gnu.org
` (10 subsequent siblings)
15 siblings, 0 replies; 17+ messages in thread
From: jakub at gcc dot gnu.org @ 2023-01-10 23:36 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108365
--- Comment #4 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
With r9-1730 or later, I think the problem is that something decides to narrow
the division from long long to int. In long long it is well defined if b is
non-zero
as -2147483648LL / -1LL is 2147483648LL. But when we instead narrow it to
-2147483648 / -1 is UB which triggers division by zero exception.
We don't seem to narrow:
int
foo (int x, int y)
{
return (long long) x / (long long) y;
}
so probably we do it only if the dividend is constant or something similar; if
yes, then the fix would be stop narrowing if the dividend is the minimum of the
narrower type.
^ permalink raw reply [flat|nested] 17+ messages in thread
* [Bug middle-end/108365] [9/10/11/12/13 Regression] Wrong code with -O0
2023-01-10 22:47 [Bug tree-optimization/108365] New: [9/10/11/12/13 Regression] Wrong code with -O0 vsevolod.livinskiy at gmail dot com
` (4 preceding siblings ...)
2023-01-10 23:36 ` jakub at gcc dot gnu.org
@ 2023-01-10 23:59 ` jakub at gcc dot gnu.org
2023-01-11 11:47 ` [Bug c++/108365] " jakub at gcc dot gnu.org
` (9 subsequent siblings)
15 siblings, 0 replies; 17+ messages in thread
From: jakub at gcc dot gnu.org @ 2023-01-10 23:59 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108365
--- Comment #5 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
I think the bug is in the C++ FE:
/* When dividing two signed integers, we have to promote to int.
unless we divide by a constant != -1. Note that default
conversion will have been performed on the operands at this
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)));
compare that to C FE, which does:
/* Although it would be tempting to shorten always here, that
loses 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)));
C FE does that only if orig_op0 was unsigned, where orig_op0 is what is passed
to the function, where op0 is perhaps later promoted. While the way it is
written in C++ FE
matches both unsigned {char,short} dividend promoted to int, but also the case
in the
testcase where orig_op0 is (long long) (unsigned long long) (-__INT_MAX__ - 1)
unfolded. If op0 is promoted from unsigned type to wider signed type or if op0
has unsigned type, then shortening is of course possible, but if op0 is
converted from unsigned type to same sized signed type or to a narrower type,
we don't know if it can't be the signed minimum.
^ permalink raw reply [flat|nested] 17+ messages in thread
* [Bug c++/108365] [9/10/11/12/13 Regression] Wrong code with -O0
2023-01-10 22:47 [Bug tree-optimization/108365] New: [9/10/11/12/13 Regression] Wrong code with -O0 vsevolod.livinskiy at gmail dot com
` (5 preceding siblings ...)
2023-01-10 23:59 ` jakub at gcc dot gnu.org
@ 2023-01-11 11:47 ` jakub at gcc dot gnu.org
2023-01-11 12:10 ` rguenth at gcc dot gnu.org
` (8 subsequent siblings)
15 siblings, 0 replies; 17+ messages in thread
From: jakub at gcc dot gnu.org @ 2023-01-11 11:47 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108365
Jakub Jelinek <jakub at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Assignee|unassigned at gcc dot gnu.org |jakub at gcc dot gnu.org
Status|NEW |ASSIGNED
--- Comment #6 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Created attachment 54243
--> https://gcc.gnu.org/bugzilla/attachment.cgi?id=54243&action=edit
gcc13-pr108365.patch
Untested fix.
^ permalink raw reply [flat|nested] 17+ messages in thread
* [Bug c++/108365] [9/10/11/12/13 Regression] Wrong code with -O0
2023-01-10 22:47 [Bug tree-optimization/108365] New: [9/10/11/12/13 Regression] Wrong code with -O0 vsevolod.livinskiy at gmail dot com
` (6 preceding siblings ...)
2023-01-11 11:47 ` [Bug c++/108365] " jakub at gcc dot gnu.org
@ 2023-01-11 12:10 ` rguenth at gcc dot gnu.org
2023-01-14 9:18 ` cvs-commit at gcc dot gnu.org
` (7 subsequent siblings)
15 siblings, 0 replies; 17+ messages in thread
From: rguenth at gcc dot gnu.org @ 2023-01-11 12:10 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108365
Richard Biener <rguenth at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Priority|P3 |P2
^ permalink raw reply [flat|nested] 17+ messages in thread
* [Bug c++/108365] [9/10/11/12/13 Regression] Wrong code with -O0
2023-01-10 22:47 [Bug tree-optimization/108365] New: [9/10/11/12/13 Regression] Wrong code with -O0 vsevolod.livinskiy at gmail dot com
` (7 preceding siblings ...)
2023-01-11 12:10 ` rguenth at gcc dot gnu.org
@ 2023-01-14 9:18 ` cvs-commit at gcc dot gnu.org
2023-01-14 9:23 ` [Bug c++/108365] [9/10/11/12 " jakub at gcc dot gnu.org
` (6 subsequent siblings)
15 siblings, 0 replies; 17+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2023-01-14 9:18 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108365
--- Comment #7 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Jakub Jelinek <jakub@gcc.gnu.org>:
https://gcc.gnu.org/g:5b3a88640f962d4ffca31ae651bed2d8672f1a8c
commit r13-5163-g5b3a88640f962d4ffca31ae651bed2d8672f1a8c
Author: Jakub Jelinek <jakub@redhat.com>
Date: Sat Jan 14 10:17:14 2023 +0100
c++: Avoid incorrect shortening of divisions [PR108365]
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.
And changes both the C and C++ FEs to do the same thing, using a helper
function in c-family.
2023-01-14 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.
^ permalink raw reply [flat|nested] 17+ messages in thread
* [Bug c++/108365] [9/10/11/12 Regression] Wrong code with -O0
2023-01-10 22:47 [Bug tree-optimization/108365] New: [9/10/11/12/13 Regression] Wrong code with -O0 vsevolod.livinskiy at gmail dot com
` (8 preceding siblings ...)
2023-01-14 9:18 ` cvs-commit at gcc dot gnu.org
@ 2023-01-14 9:23 ` jakub at gcc dot gnu.org
2023-02-10 17:47 ` cvs-commit at gcc dot gnu.org
` (5 subsequent siblings)
15 siblings, 0 replies; 17+ messages in thread
From: jakub at gcc dot gnu.org @ 2023-01-14 9:23 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108365
Jakub Jelinek <jakub at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Summary|[9/10/11/12/13 Regression] |[9/10/11/12 Regression]
|Wrong code with -O0 |Wrong code with -O0
--- Comment #8 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Fixed on the trunk so far.
Guess for backports we want instead a minimal change (i.e. just the
+ && INTEGRAL_TYPE_P (TREE_TYPE (TREE_OPERAND (op0, 0)))
and
+ && (TYPE_PRECISION (TREE_TYPE (TREE_OPERAND (op0, 0)))
+ < TYPE_PRECISION (type0)))
additions for C++ FE).
^ permalink raw reply [flat|nested] 17+ messages in thread
* [Bug c++/108365] [9/10/11/12 Regression] Wrong code with -O0
2023-01-10 22:47 [Bug tree-optimization/108365] New: [9/10/11/12/13 Regression] Wrong code with -O0 vsevolod.livinskiy at gmail dot com
` (9 preceding siblings ...)
2023-01-14 9:23 ` [Bug c++/108365] [9/10/11/12 " jakub at gcc dot gnu.org
@ 2023-02-10 17:47 ` cvs-commit at gcc dot gnu.org
2023-02-10 18:00 ` [Bug c++/108365] [10/11 " jakub at gcc dot gnu.org
` (4 subsequent siblings)
15 siblings, 0 replies; 17+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2023-02-10 17:47 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108365
--- Comment #9 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-12 branch has been updated by Jakub Jelinek
<jakub@gcc.gnu.org>:
https://gcc.gnu.org/g:369454ecb53a2911946356b09347259c953f435f
commit r12-9156-g369454ecb53a2911946356b09347259c953f435f
Author: Jakub Jelinek <jakub@redhat.com>
Date: Sat Jan 14 10:17:14 2023 +0100
c++: Avoid incorrect shortening of divisions [PR108365]
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.
2023-01-14 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.
(cherry picked from commit 5b3a88640f962d4ffca31ae651bed2d8672f1a8c)
^ permalink raw reply [flat|nested] 17+ messages in thread
* [Bug c++/108365] [10/11 Regression] Wrong code with -O0
2023-01-10 22:47 [Bug tree-optimization/108365] New: [9/10/11/12/13 Regression] Wrong code with -O0 vsevolod.livinskiy at gmail dot com
` (10 preceding siblings ...)
2023-02-10 17:47 ` cvs-commit at gcc dot gnu.org
@ 2023-02-10 18:00 ` jakub at gcc dot gnu.org
2023-05-02 20:14 ` cvs-commit at gcc dot gnu.org
` (3 subsequent siblings)
15 siblings, 0 replies; 17+ messages in thread
From: jakub at gcc dot gnu.org @ 2023-02-10 18:00 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108365
Jakub Jelinek <jakub at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Summary|[9/10/11/12 Regression] |[10/11 Regression] Wrong
|Wrong code with -O0 |code with -O0
--- Comment #10 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Fixed for gcc 12.3 too.
^ permalink raw reply [flat|nested] 17+ messages in thread
* [Bug c++/108365] [10/11 Regression] Wrong code with -O0
2023-01-10 22:47 [Bug tree-optimization/108365] New: [9/10/11/12/13 Regression] Wrong code with -O0 vsevolod.livinskiy at gmail dot com
` (11 preceding siblings ...)
2023-02-10 18:00 ` [Bug c++/108365] [10/11 " jakub at gcc dot gnu.org
@ 2023-05-02 20:14 ` cvs-commit at gcc dot gnu.org
2023-05-03 9:37 ` [Bug c++/108365] [10 " jakub at gcc dot gnu.org
` (2 subsequent siblings)
15 siblings, 0 replies; 17+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2023-05-02 20:14 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108365
--- Comment #11 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-11 branch has been updated by Jakub Jelinek
<jakub@gcc.gnu.org>:
https://gcc.gnu.org/g:77df8c7fc78b4ea7dc340f9f0fc239032ce0f9dd
commit r11-10711-g77df8c7fc78b4ea7dc340f9f0fc239032ce0f9dd
Author: Jakub Jelinek <jakub@redhat.com>
Date: Sat Jan 14 10:17:14 2023 +0100
c++: Avoid incorrect shortening of divisions [PR108365]
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.
2023-01-14 Jakub Jelinek <jakub@redhat.com>
PR c++/108365
* typeck.c (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.
(cherry picked from commit 5b3a88640f962d4ffca31ae651bed2d8672f1a8c)
^ permalink raw reply [flat|nested] 17+ messages in thread
* [Bug c++/108365] [10 Regression] Wrong code with -O0
2023-01-10 22:47 [Bug tree-optimization/108365] New: [9/10/11/12/13 Regression] Wrong code with -O0 vsevolod.livinskiy at gmail dot com
` (12 preceding siblings ...)
2023-05-02 20:14 ` cvs-commit at gcc dot gnu.org
@ 2023-05-03 9:37 ` jakub at gcc dot gnu.org
2023-05-03 15:21 ` cvs-commit at gcc dot gnu.org
2023-05-04 7:23 ` jakub at gcc dot gnu.org
15 siblings, 0 replies; 17+ messages in thread
From: jakub at gcc dot gnu.org @ 2023-05-03 9:37 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108365
Jakub Jelinek <jakub at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Summary|[10/11 Regression] Wrong |[10 Regression] Wrong code
|code with -O0 |with -O0
--- Comment #12 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Fixed for 11.4 as well.
^ permalink raw reply [flat|nested] 17+ messages in thread
* [Bug c++/108365] [10 Regression] Wrong code with -O0
2023-01-10 22:47 [Bug tree-optimization/108365] New: [9/10/11/12/13 Regression] Wrong code with -O0 vsevolod.livinskiy at gmail dot com
` (13 preceding siblings ...)
2023-05-03 9:37 ` [Bug c++/108365] [10 " jakub at gcc dot gnu.org
@ 2023-05-03 15:21 ` cvs-commit at gcc dot gnu.org
2023-05-04 7:23 ` jakub at gcc dot gnu.org
15 siblings, 0 replies; 17+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2023-05-03 15:21 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108365
--- Comment #13 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-10 branch has been updated by Jakub Jelinek
<jakub@gcc.gnu.org>:
https://gcc.gnu.org/g:61948b4113a817b0cd61e7e5311d9cfae31bc623
commit r10-11365-g61948b4113a817b0cd61e7e5311d9cfae31bc623
Author: Jakub Jelinek <jakub@redhat.com>
Date: Sat Jan 14 10:17:14 2023 +0100
c++: Avoid incorrect shortening of divisions [PR108365]
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.
2023-01-14 Jakub Jelinek <jakub@redhat.com>
PR c++/108365
* typeck.c (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.
(cherry picked from commit 5b3a88640f962d4ffca31ae651bed2d8672f1a8c)
^ permalink raw reply [flat|nested] 17+ messages in thread
* [Bug c++/108365] [10 Regression] Wrong code with -O0
2023-01-10 22:47 [Bug tree-optimization/108365] New: [9/10/11/12/13 Regression] Wrong code with -O0 vsevolod.livinskiy at gmail dot com
` (14 preceding siblings ...)
2023-05-03 15:21 ` cvs-commit at gcc dot gnu.org
@ 2023-05-04 7:23 ` jakub at gcc dot gnu.org
15 siblings, 0 replies; 17+ messages in thread
From: jakub at gcc dot gnu.org @ 2023-05-04 7:23 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108365
Jakub Jelinek <jakub at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|ASSIGNED |RESOLVED
Resolution|--- |FIXED
--- Comment #14 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Fixed for 10.5 too.
^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2023-05-04 7:23 UTC | newest]
Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-01-10 22:47 [Bug tree-optimization/108365] New: [9/10/11/12/13 Regression] Wrong code with -O0 vsevolod.livinskiy at gmail dot com
2023-01-10 22:55 ` [Bug tree-optimization/108365] " pinskia at gcc dot gnu.org
2023-01-10 22:55 ` pinskia at gcc dot gnu.org
2023-01-10 23:07 ` [Bug middle-end/108365] " jakub at gcc dot gnu.org
2023-01-10 23:15 ` pinskia at gcc dot gnu.org
2023-01-10 23:36 ` jakub at gcc dot gnu.org
2023-01-10 23:59 ` jakub at gcc dot gnu.org
2023-01-11 11:47 ` [Bug c++/108365] " jakub at gcc dot gnu.org
2023-01-11 12:10 ` rguenth at gcc dot gnu.org
2023-01-14 9:18 ` cvs-commit at gcc dot gnu.org
2023-01-14 9:23 ` [Bug c++/108365] [9/10/11/12 " jakub at gcc dot gnu.org
2023-02-10 17:47 ` cvs-commit at gcc dot gnu.org
2023-02-10 18:00 ` [Bug c++/108365] [10/11 " jakub at gcc dot gnu.org
2023-05-02 20:14 ` cvs-commit at gcc dot gnu.org
2023-05-03 9:37 ` [Bug c++/108365] [10 " jakub at gcc dot gnu.org
2023-05-03 15:21 ` cvs-commit at gcc dot gnu.org
2023-05-04 7:23 ` jakub at gcc dot gnu.org
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).