public inbox for gcc-bugs@sourceware.org
help / color / mirror / Atom feed
* [Bug tree-optimization/109691] New: Takes until forwprop2 to remove !a sometimes
@ 2023-05-02 5:06 pinskia at gcc dot gnu.org
2023-05-02 6:31 ` [Bug tree-optimization/109691] " pinskia at gcc dot gnu.org
` (7 more replies)
0 siblings, 8 replies; 9+ messages in thread
From: pinskia at gcc dot gnu.org @ 2023-05-02 5:06 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109691
Bug ID: 109691
Summary: Takes until forwprop2 to remove !a sometimes
Product: gcc
Version: 14.0
Status: UNCONFIRMED
Keywords: internal-improvement, missed-optimization
Severity: normal
Priority: P3
Component: tree-optimization
Assignee: unassigned at gcc dot gnu.org
Reporter: pinskia at gcc dot gnu.org
Target Milestone: ---
Take:
```
int f(_Bool c, int a, int b)
{
_Bool d = c;
d = !d;
if (d != 0) return a;
return b;
}
```
It takes until forwprop2 now to remove all of the code before the conditional.
In GCC 4.9.0, we used to do it almost all the way in forwprop1; with just one
assignment added.
I noticed this while looking into PR 67628 but it is not exactly an issue there
so filing it seperately.
^ permalink raw reply [flat|nested] 9+ messages in thread
* [Bug tree-optimization/109691] Takes until forwprop2 to remove !a sometimes
2023-05-02 5:06 [Bug tree-optimization/109691] New: Takes until forwprop2 to remove !a sometimes pinskia at gcc dot gnu.org
@ 2023-05-02 6:31 ` pinskia at gcc dot gnu.org
2023-05-02 23:58 ` pinskia at gcc dot gnu.org
` (6 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: pinskia at gcc dot gnu.org @ 2023-05-02 6:31 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109691
Andrew Pinski <pinskia at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|UNCONFIRMED |ASSIGNED
Last reconfirmed| |2023-05-02
Ever confirmed|0 |1
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 have a patch, basically moves tree-ssa-propagate to use
simple_dce_from_worklist instead of its own statement removal worklist. As that
will do it recusively find dead statements.
^ permalink raw reply [flat|nested] 9+ messages in thread
* [Bug tree-optimization/109691] Takes until forwprop2 to remove !a sometimes
2023-05-02 5:06 [Bug tree-optimization/109691] New: Takes until forwprop2 to remove !a sometimes pinskia at gcc dot gnu.org
2023-05-02 6:31 ` [Bug tree-optimization/109691] " pinskia at gcc dot gnu.org
@ 2023-05-02 23:58 ` pinskia at gcc dot gnu.org
2023-05-03 3:25 ` pinskia at gcc dot gnu.org
` (5 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: pinskia at gcc dot gnu.org @ 2023-05-02 23:58 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109691
--- Comment #2 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Still working on this but I ran into an issue where we now remove a dead load
but that load has an exception edge (due to -fnon-call-exceptions) and we don't
cleanup the eh edge and get an ICE due to that.
FAIL: g++.dg/pr55263.C -std=gnu++14 (internal compiler error: verify_flow_info
failed)
FAIL: g++.dg/tree-ssa/pr36766.C -std=gnu++14 (internal compiler error:
verify_flow_info failed)
Also still have to update a few other testcases.
FAIL: gcc.dg/pr81192.c scan-tree-dump-times pre "(?n)find_duplicates: <bb .*>
duplicate of <bb .*>" 1
FAIL: c-c++-common/goacc/kernels-alias-8.c scan-tree-dump-times ealias "clique
1 base 1" 2
kernels-alias-8.c is because we also dce a load I have to double check how to
handle that one but -fno-tree-ccp might work ...
gcc.dg/pr81192.c might need a similar thing now too but I am not 100% sure.
pr81192.c should really be a Gimple testcase but gimple testcases were not
around when the testcase was added.
^ permalink raw reply [flat|nested] 9+ messages in thread
* [Bug tree-optimization/109691] Takes until forwprop2 to remove !a sometimes
2023-05-02 5:06 [Bug tree-optimization/109691] New: Takes until forwprop2 to remove !a sometimes pinskia at gcc dot gnu.org
2023-05-02 6:31 ` [Bug tree-optimization/109691] " pinskia at gcc dot gnu.org
2023-05-02 23:58 ` pinskia at gcc dot gnu.org
@ 2023-05-03 3:25 ` pinskia at gcc dot gnu.org
2023-05-05 5:16 ` pinskia at gcc dot gnu.org
` (4 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: pinskia at gcc dot gnu.org @ 2023-05-03 3:25 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109691
--- Comment #3 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
(In reply to Andrew Pinski from comment #2)
> Still working on this but I ran into an issue where we now remove a dead
> load but that load has an exception edge (due to -fnon-call-exceptions) and
> we don't cleanup the eh edge and get an ICE due to that.
>
> FAIL: g++.dg/pr55263.C -std=gnu++14 (internal compiler error:
> verify_flow_info failed)
>
> FAIL: g++.dg/tree-ssa/pr36766.C -std=gnu++14 (internal compiler error:
> verify_flow_info failed)
I have a fix, it was simplier than expected.
^ permalink raw reply [flat|nested] 9+ messages in thread
* [Bug tree-optimization/109691] Takes until forwprop2 to remove !a sometimes
2023-05-02 5:06 [Bug tree-optimization/109691] New: Takes until forwprop2 to remove !a sometimes pinskia at gcc dot gnu.org
` (2 preceding siblings ...)
2023-05-03 3:25 ` pinskia at gcc dot gnu.org
@ 2023-05-05 5:16 ` pinskia at gcc dot gnu.org
2023-05-05 6:09 ` pinskia at gcc dot gnu.org
` (3 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: pinskia at gcc dot gnu.org @ 2023-05-05 5:16 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109691
--- Comment #4 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
(In reply to Andrew Pinski from comment #2)
> gcc.dg/pr81192.c might need a similar thing now too but I am not 100% sure.
> pr81192.c should really be a Gimple testcase but gimple testcases were not
> around when the testcase was added.
It is going to need to be a gimple testcase. The testcase has become too
fragile otherwise; it already has gone through a few -fno-* -fdisable-* options
already too.
^ permalink raw reply [flat|nested] 9+ messages in thread
* [Bug tree-optimization/109691] Takes until forwprop2 to remove !a sometimes
2023-05-02 5:06 [Bug tree-optimization/109691] New: Takes until forwprop2 to remove !a sometimes pinskia at gcc dot gnu.org
` (3 preceding siblings ...)
2023-05-05 5:16 ` pinskia at gcc dot gnu.org
@ 2023-05-05 6:09 ` pinskia at gcc dot gnu.org
2023-05-05 15:18 ` pinskia at gcc dot gnu.org
` (2 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: pinskia at gcc dot gnu.org @ 2023-05-05 6:09 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109691
--- Comment #5 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Created attachment 55003
--> https://gcc.gnu.org/bugzilla/attachment.cgi?id=55003&action=edit
Patch which is under testing including updates to the testsuite finally
^ permalink raw reply [flat|nested] 9+ messages in thread
* [Bug tree-optimization/109691] Takes until forwprop2 to remove !a sometimes
2023-05-02 5:06 [Bug tree-optimization/109691] New: Takes until forwprop2 to remove !a sometimes pinskia at gcc dot gnu.org
` (4 preceding siblings ...)
2023-05-05 6:09 ` pinskia at gcc dot gnu.org
@ 2023-05-05 15:18 ` pinskia at gcc dot gnu.org
2023-05-08 6:18 ` cvs-commit at gcc dot gnu.org
2023-05-08 6:18 ` pinskia at gcc dot gnu.org
7 siblings, 0 replies; 9+ messages in thread
From: pinskia at gcc dot gnu.org @ 2023-05-05 15:18 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109691
Andrew Pinski <pinskia at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Keywords| |patch
URL| |https://gcc.gnu.org/piperma
| |il/gcc-patches/2023-May/617
| |612.html
--- Comment #6 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Patch submitted:
https://gcc.gnu.org/pipermail/gcc-patches/2023-May/617612.html
^ permalink raw reply [flat|nested] 9+ messages in thread
* [Bug tree-optimization/109691] Takes until forwprop2 to remove !a sometimes
2023-05-02 5:06 [Bug tree-optimization/109691] New: Takes until forwprop2 to remove !a sometimes pinskia at gcc dot gnu.org
` (5 preceding siblings ...)
2023-05-05 15:18 ` pinskia at gcc dot gnu.org
@ 2023-05-08 6:18 ` cvs-commit at gcc dot gnu.org
2023-05-08 6:18 ` pinskia at gcc dot gnu.org
7 siblings, 0 replies; 9+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2023-05-08 6:18 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109691
--- Comment #7 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The trunk branch has been updated by Andrew Pinski <pinskia@gcc.gnu.org>:
https://gcc.gnu.org/g:21e2ef2dc25de318de29ec32d5390350c6717c6a
commit r14-569-g21e2ef2dc25de318de29ec32d5390350c6717c6a
Author: Andrew Pinski <apinski@marvell.com>
Date: Tue May 2 00:10:46 2023 -0700
Move substitute_and_fold over to use simple_dce_from_worklist
While looking into a different issue, I noticed that it
would take until the second forwprop pass to do some
forward proping and it was because the ssa name was
used more than once but the second statement was
"dead" and we don't remove that until much later.
So this uses simple_dce_from_worklist instead of manually
removing of the known unused statements instead.
Propagate engine does not do a cleanupcfg afterwards either but manually
cleans up possible EH edges so simple_dce_from_worklist
needs to communicate that back to the propagate engine.
Some testcases needed to be updated/changed even because of better
optimization.
gcc.dg/pr81192.c even had to be changed to be using the gimple FE so it
would
be less fragile in the future too.
gcc.dg/tree-ssa/pr98737-1.c was failing because __atomic_fetch_ was being
matched
but in those cases, the result was not being used so both __atomic_fetch_
and
__atomic_x_and_fetch_ are valid choices and would not make a code
generation difference.
evrp7.c, evrp8.c, vrp35.c, vrp36.c: just needed a slightly change as the
removal message
is different slightly.
kernels-alias-8.c: ccp1 is able to remove an unused load which causes
ealias to have
one less load to analysis so update the expected scan #.
OK? Bootstrapped and tested on x86_64-linux-gnu with no regressions.
gcc/ChangeLog:
PR tree-optimization/109691
* tree-ssa-dce.cc (simple_dce_from_worklist): Add need_eh_cleanup
argument.
If the removed statement can throw, have need_eh_cleanup
include the bb of that statement.
* tree-ssa-dce.h (simple_dce_from_worklist): Update declaration.
* tree-ssa-propagate.cc (struct prop_stats_d): Remove
num_dce.
(substitute_and_fold_dom_walker::substitute_and_fold_dom_walker):
Initialize dceworklist instead of stmts_to_remove.
(substitute_and_fold_dom_walker::~substitute_and_fold_dom_walker):
Destore dceworklist instead of stmts_to_remove.
(substitute_and_fold_dom_walker::before_dom_children):
Set dceworklist instead of adding to stmts_to_remove.
(substitute_and_fold_engine::substitute_and_fold):
Call simple_dce_from_worklist instead of poping
from the list.
Don't update the stat on removal statements.
gcc/testsuite/ChangeLog:
* gcc.dg/tree-ssa/evrp7.c: Update for output change.
* gcc.dg/tree-ssa/evrp8.c: Likewise.
* gcc.dg/tree-ssa/vrp35.c: Likewise.
* gcc.dg/tree-ssa/vrp36.c: Likewise.
* gcc.dg/tree-ssa/pr98737-1.c: Update scan-tree-dump-not
to check for assignment too instead of just a call.
* c-c++-common/goacc/kernels-alias-8.c: Update test
for removal of load.
* gcc.dg/pr81192.c: Rewrite testcase in gimple based test.
^ permalink raw reply [flat|nested] 9+ messages in thread
* [Bug tree-optimization/109691] Takes until forwprop2 to remove !a sometimes
2023-05-02 5:06 [Bug tree-optimization/109691] New: Takes until forwprop2 to remove !a sometimes pinskia at gcc dot gnu.org
` (6 preceding siblings ...)
2023-05-08 6:18 ` cvs-commit at gcc dot gnu.org
@ 2023-05-08 6:18 ` pinskia at gcc dot gnu.org
7 siblings, 0 replies; 9+ messages in thread
From: pinskia at gcc dot gnu.org @ 2023-05-08 6:18 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109691
Andrew Pinski <pinskia at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|ASSIGNED |RESOLVED
Target Milestone|--- |14.0
Resolution|--- |FIXED
--- Comment #8 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Fixed.
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2023-05-08 6:18 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-05-02 5:06 [Bug tree-optimization/109691] New: Takes until forwprop2 to remove !a sometimes pinskia at gcc dot gnu.org
2023-05-02 6:31 ` [Bug tree-optimization/109691] " pinskia at gcc dot gnu.org
2023-05-02 23:58 ` pinskia at gcc dot gnu.org
2023-05-03 3:25 ` pinskia at gcc dot gnu.org
2023-05-05 5:16 ` pinskia at gcc dot gnu.org
2023-05-05 6:09 ` pinskia at gcc dot gnu.org
2023-05-05 15:18 ` pinskia at gcc dot gnu.org
2023-05-08 6:18 ` cvs-commit at gcc dot gnu.org
2023-05-08 6:18 ` pinskia 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).