public inbox for gcc-bugs@sourceware.org
help / color / mirror / Atom feed
* [Bug tree-optimization/104645] New: [12 Regression] i ? i % 2 : 0 not optimized anymore
@ 2022-02-22 19:13 denis.campredon at gmail dot com
2022-02-22 19:16 ` [Bug tree-optimization/104645] " pinskia at gcc dot gnu.org
` (8 more replies)
0 siblings, 9 replies; 10+ messages in thread
From: denis.campredon at gmail dot com @ 2022-02-22 19:13 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104645
Bug ID: 104645
Summary: [12 Regression] i ? i % 2 : 0 not optimized anymore
Product: gcc
Version: 12.0
Status: UNCONFIRMED
Severity: normal
Priority: P3
Component: tree-optimization
Assignee: unassigned at gcc dot gnu.org
Reporter: denis.campredon at gmail dot com
Target Milestone: ---
Was told to file a new PR for that case :
------------------
int foo(unsigned i) {
return i ? i % 2 : 0;
}
------------------
With trunk
------------------------
foo(unsigned int):
mov eax, edi
xor edx, edx
and eax, 1
test edi, edi
cmove eax, edx
ret
-----------------------
With 11.2
-----------------------
foo(unsigned int):
mov eax, edi
and eax, 1
ret
-----------------------
According to Jakub Jelinek in PR104639 it started with
r12-5358-g32221357007666124409ec3ee0d3a1cf263ebc9e
^ permalink raw reply [flat|nested] 10+ messages in thread
* [Bug tree-optimization/104645] [12 Regression] i ? i % 2 : 0 not optimized anymore
2022-02-22 19:13 [Bug tree-optimization/104645] New: [12 Regression] i ? i % 2 : 0 not optimized anymore denis.campredon at gmail dot com
@ 2022-02-22 19:16 ` pinskia at gcc dot gnu.org
2022-02-22 19:17 ` jakub at gcc dot gnu.org
` (7 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: pinskia at gcc dot gnu.org @ 2022-02-22 19:16 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104645
Andrew Pinski <pinskia at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Target Milestone|--- |12.0
Keywords| |missed-optimization
CC| |pinskia at gcc dot gnu.org
^ permalink raw reply [flat|nested] 10+ messages in thread
* [Bug tree-optimization/104645] [12 Regression] i ? i % 2 : 0 not optimized anymore
2022-02-22 19:13 [Bug tree-optimization/104645] New: [12 Regression] i ? i % 2 : 0 not optimized anymore denis.campredon at gmail dot com
2022-02-22 19:16 ` [Bug tree-optimization/104645] " pinskia at gcc dot gnu.org
@ 2022-02-22 19:17 ` jakub at gcc dot gnu.org
2022-02-22 20:15 ` pinskia at gcc dot gnu.org
` (6 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: jakub at gcc dot gnu.org @ 2022-02-22 19:17 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104645
Jakub Jelinek <jakub at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Priority|P3 |P1
Ever confirmed|0 |1
CC| |jakub at gcc dot gnu.org
Last reconfirmed| |2022-02-22
Status|UNCONFIRMED |NEW
^ permalink raw reply [flat|nested] 10+ messages in thread
* [Bug tree-optimization/104645] [12 Regression] i ? i % 2 : 0 not optimized anymore
2022-02-22 19:13 [Bug tree-optimization/104645] New: [12 Regression] i ? i % 2 : 0 not optimized anymore denis.campredon at gmail dot com
2022-02-22 19:16 ` [Bug tree-optimization/104645] " pinskia at gcc dot gnu.org
2022-02-22 19:17 ` jakub at gcc dot gnu.org
@ 2022-02-22 20:15 ` pinskia at gcc dot gnu.org
2022-02-22 20:18 ` pinskia at gcc dot gnu.org
` (5 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: pinskia at gcc dot gnu.org @ 2022-02-22 20:15 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104645
Andrew Pinski <pinskia at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Assignee|unassigned at gcc dot gnu.org |pinskia at gcc dot gnu.org
--- Comment #1 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
I don't think this should be a P1.
Here is another testcase which shows the same issue but is not a regression:
int foo(unsigned i) {
int t = 0;
if (i)
{
unsigned tt = i % 2u;
t = tt;
}
return t;
}
int foo1(unsigned i) {
unsigned t = i % 2;
unsigned tt = i ? t : 0;
return tt;
}
---- CUT ----
For some reason the code in value_replacement is not able to handle the
different IR.
I have some ideas on how to fix this for GCC 13 though.
^ permalink raw reply [flat|nested] 10+ messages in thread
* [Bug tree-optimization/104645] [12 Regression] i ? i % 2 : 0 not optimized anymore
2022-02-22 19:13 [Bug tree-optimization/104645] New: [12 Regression] i ? i % 2 : 0 not optimized anymore denis.campredon at gmail dot com
` (2 preceding siblings ...)
2022-02-22 20:15 ` pinskia at gcc dot gnu.org
@ 2022-02-22 20:18 ` pinskia at gcc dot gnu.org
2022-02-23 8:23 ` rguenth at gcc dot gnu.org
` (4 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: pinskia at gcc dot gnu.org @ 2022-02-22 20:18 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104645
Andrew Pinski <pinskia at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|NEW |ASSIGNED
^ permalink raw reply [flat|nested] 10+ messages in thread
* [Bug tree-optimization/104645] [12 Regression] i ? i % 2 : 0 not optimized anymore
2022-02-22 19:13 [Bug tree-optimization/104645] New: [12 Regression] i ? i % 2 : 0 not optimized anymore denis.campredon at gmail dot com
` (3 preceding siblings ...)
2022-02-22 20:18 ` pinskia at gcc dot gnu.org
@ 2022-02-23 8:23 ` rguenth at gcc dot gnu.org
2022-03-31 13:09 ` jakub at gcc dot gnu.org
` (3 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: rguenth at gcc dot gnu.org @ 2022-02-23 8:23 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104645
--- Comment #2 from Richard Biener <rguenth at gcc dot gnu.org> ---
So it's unhandled
<bb 2> :
if (i_3(D) != 0)
goto <bb 3>; [INV]
else
goto <bb 4>; [INV]
<bb 3> :
_4 = i_3(D) & 1;
iftmp.0_5 = (int) _4;
<bb 4> :
# iftmp.0_2 = PHI <iftmp.0_5(3), 0(2)>
return iftmp.0_2;
it also doesn't handle
<bb 2> :
if (i_3(D) != 0)
goto <bb 3>; [INV]
else
goto <bb 4>; [INV]
<bb 3> :
i.1_1 = (int) i_3(D);
iftmp.0_5 = i.1_1 & 1;
<bb 4> :
# iftmp.0_2 = PHI <iftmp.0_5(3), 0(2)>
return iftmp.0_2;
I wonder if we can't cook up a fix for GCC 12 though since it _is_ a regression
after all. Just look through (nop-)conversions somehow.
^ permalink raw reply [flat|nested] 10+ messages in thread
* [Bug tree-optimization/104645] [12 Regression] i ? i % 2 : 0 not optimized anymore
2022-02-22 19:13 [Bug tree-optimization/104645] New: [12 Regression] i ? i % 2 : 0 not optimized anymore denis.campredon at gmail dot com
` (4 preceding siblings ...)
2022-02-23 8:23 ` rguenth at gcc dot gnu.org
@ 2022-03-31 13:09 ` jakub at gcc dot gnu.org
2022-03-31 13:10 ` jakub at gcc dot gnu.org
` (2 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: jakub at gcc dot gnu.org @ 2022-03-31 13:09 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104645
--- Comment #3 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Created attachment 52725
--> https://gcc.gnu.org/bugzilla/attachment.cgi?id=52725&action=edit
gcc12-pr104645.patch
I wonder if at least for GCC 13 we just can't treat even that cast stmt as a
preparation statement.
This won't handle say shifts etc. with casts after them, but will at least fix
this regression.
And for GCC 13 perhaps we can throw away all the separation of "preparation"
and assign statements and natural_element_p and absorbing_element_p and instead
just try to constant evaluate all the middle_bb statements and see if that
doesn't trigger traps/overflows/other UB and yields the expected PHI arg
constant from the original comparison constant.
^ permalink raw reply [flat|nested] 10+ messages in thread
* [Bug tree-optimization/104645] [12 Regression] i ? i % 2 : 0 not optimized anymore
2022-02-22 19:13 [Bug tree-optimization/104645] New: [12 Regression] i ? i % 2 : 0 not optimized anymore denis.campredon at gmail dot com
` (5 preceding siblings ...)
2022-03-31 13:09 ` jakub at gcc dot gnu.org
@ 2022-03-31 13:10 ` jakub at gcc dot gnu.org
2022-04-01 9:51 ` cvs-commit at gcc dot gnu.org
2022-04-01 9:52 ` jakub at gcc dot gnu.org
8 siblings, 0 replies; 10+ messages in thread
From: jakub at gcc dot gnu.org @ 2022-03-31 13:10 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104645
--- Comment #4 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
The first GCC 13 should have been GCC 12 of course.
^ permalink raw reply [flat|nested] 10+ messages in thread
* [Bug tree-optimization/104645] [12 Regression] i ? i % 2 : 0 not optimized anymore
2022-02-22 19:13 [Bug tree-optimization/104645] New: [12 Regression] i ? i % 2 : 0 not optimized anymore denis.campredon at gmail dot com
` (6 preceding siblings ...)
2022-03-31 13:10 ` jakub at gcc dot gnu.org
@ 2022-04-01 9:51 ` cvs-commit at gcc dot gnu.org
2022-04-01 9:52 ` jakub at gcc dot gnu.org
8 siblings, 0 replies; 10+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2022-04-01 9:51 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104645
--- Comment #5 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:d9c03fc27d8147a9401a29739694b214df48a9a2
commit r12-7952-gd9c03fc27d8147a9401a29739694b214df48a9a2
Author: Jakub Jelinek <jakub@redhat.com>
Date: Fri Apr 1 11:50:41 2022 +0200
phiopt: Improve value_replacement [PR104645]
The following patch fixes the P1 regression by reusing existing
value_replacement code. That function already has code to
handle simple preparation statements (casts, and +,&,|,^ binary
assignments) before a final binary assignment (which can be
much wider range of ops). When we have e.g.
if (y_3(D) == 0)
goto <bb 4>;
else
goto <bb 3>;
<bb 3>:
y_4 = y_3(D) & 31;
_1 = (int) y_4;
_6 = x_5(D) r<< _1;
<bb 4>:
# _2 = PHI <x_5(D)(2), _6(3)>
the preparation statements y_4 = y_3(D) & 31; and
_1 = (int) y_4; are handled by constant evaluation, passing through
y_3(D) = 0 initially and propagating that through the assignments
with checking that UB isn't invoked. But the final
_6 = x_5(D) r<< _1; assign is handled differently, either through
neutral_element_p or absorbing_element_p.
In the first function below we now have:
<bb 2> [local count: 1073741824]:
if (i_2(D) != 0)
goto <bb 3>; [50.00%]
else
goto <bb 4>; [50.00%]
<bb 3> [local count: 536870913]:
_3 = i_2(D) & 1;
iftmp.0_4 = (int) _3;
<bb 4> [local count: 1073741824]:
# iftmp.0_1 = PHI <iftmp.0_4(3), 0(2)>
where in GCC 11 we had:
<bb 2> :
if (i_3(D) != 0)
goto <bb 3>; [INV]
else
goto <bb 4>; [INV]
<bb 3> :
i.1_1 = (int) i_3(D);
iftmp.0_5 = i.1_1 & 1;
<bb 4> :
# iftmp.0_2 = PHI <iftmp.0_5(3), 0(2)>
Current value_replacement can handle the latter as the last
stmt of middle_bb is a binary op that in this case satisfies
absorbing_element_p.
But the former we can't handle, as the last stmt in middle_bb
is a cast.
The patch makes it work in that case by pretending all of middle_bb
are the preparation statements and there is no binary assign at the
end, so everything is handled through the constant evaluation.
We simply set at the start of middle_bb the lhs of comparison
virtually to the rhs, propagate it through and at the end
see if virtually the arg0 of the PHI is equal to arg1 of it.
For GCC 13, I think we just should throw away all the neutral/absorbing
element stuff and do the constant evaluation of the whole middle_bb
and handle that way all the ops we currently handle in neutral/absorbing
element.
2022-04-01 Jakub Jelinek <jakub@redhat.com>
PR tree-optimization/104645
* tree-ssa-phiopt.cc (value_replacement): If assign has
CONVERT_EXPR_CODE_P rhs_code, treat it like a preparation
statement with constant evaluation.
* gcc.dg/tree-ssa/pr104645.c: New test.
^ permalink raw reply [flat|nested] 10+ messages in thread
* [Bug tree-optimization/104645] [12 Regression] i ? i % 2 : 0 not optimized anymore
2022-02-22 19:13 [Bug tree-optimization/104645] New: [12 Regression] i ? i % 2 : 0 not optimized anymore denis.campredon at gmail dot com
` (7 preceding siblings ...)
2022-04-01 9:51 ` cvs-commit at gcc dot gnu.org
@ 2022-04-01 9:52 ` jakub at gcc dot gnu.org
8 siblings, 0 replies; 10+ messages in thread
From: jakub at gcc dot gnu.org @ 2022-04-01 9:52 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104645
Jakub Jelinek <jakub at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Resolution|--- |FIXED
Status|ASSIGNED |RESOLVED
--- Comment #6 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Fixed.
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2022-04-01 9:52 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-02-22 19:13 [Bug tree-optimization/104645] New: [12 Regression] i ? i % 2 : 0 not optimized anymore denis.campredon at gmail dot com
2022-02-22 19:16 ` [Bug tree-optimization/104645] " pinskia at gcc dot gnu.org
2022-02-22 19:17 ` jakub at gcc dot gnu.org
2022-02-22 20:15 ` pinskia at gcc dot gnu.org
2022-02-22 20:18 ` pinskia at gcc dot gnu.org
2022-02-23 8:23 ` rguenth at gcc dot gnu.org
2022-03-31 13:09 ` jakub at gcc dot gnu.org
2022-03-31 13:10 ` jakub at gcc dot gnu.org
2022-04-01 9:51 ` cvs-commit at gcc dot gnu.org
2022-04-01 9:52 ` 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).