* [Bug tree-optimization/96722] [8/9/10/11 Regression] Clobbers on NULL since r8-1519
2020-08-20 9:30 [Bug tree-optimization/96722] New: [8/9/10/11 Regression] Clobbers on NULL since r8-1519 jakub at gcc dot gnu.org
@ 2020-08-20 9:30 ` jakub at gcc dot gnu.org
2020-08-20 9:39 ` marxin at gcc dot gnu.org
` (12 subsequent siblings)
13 siblings, 0 replies; 15+ messages in thread
From: jakub at gcc dot gnu.org @ 2020-08-20 9:30 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96722
Jakub Jelinek <jakub at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Ever confirmed|0 |1
Last reconfirmed| |2020-08-20
Target Milestone|--- |8.5
CC| |marxin at gcc dot gnu.org
Keywords|needs-bisection, |
|needs-reduction |
Status|UNCONFIRMED |NEW
^ permalink raw reply [flat|nested] 15+ messages in thread* [Bug tree-optimization/96722] [8/9/10/11 Regression] Clobbers on NULL since r8-1519
2020-08-20 9:30 [Bug tree-optimization/96722] New: [8/9/10/11 Regression] Clobbers on NULL since r8-1519 jakub at gcc dot gnu.org
2020-08-20 9:30 ` [Bug tree-optimization/96722] " jakub at gcc dot gnu.org
@ 2020-08-20 9:39 ` marxin at gcc dot gnu.org
2020-08-20 13:59 ` marxin at gcc dot gnu.org
` (11 subsequent siblings)
13 siblings, 0 replies; 15+ messages in thread
From: marxin at gcc dot gnu.org @ 2020-08-20 9:39 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96722
Martin Liška <marxin at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|NEW |ASSIGNED
Assignee|unassigned at gcc dot gnu.org |marxin at gcc dot gnu.org
--- Comment #1 from Martin Liška <marxin at gcc dot gnu.org> ---
Let me take a look.
^ permalink raw reply [flat|nested] 15+ messages in thread* [Bug tree-optimization/96722] [8/9/10/11 Regression] Clobbers on NULL since r8-1519
2020-08-20 9:30 [Bug tree-optimization/96722] New: [8/9/10/11 Regression] Clobbers on NULL since r8-1519 jakub at gcc dot gnu.org
2020-08-20 9:30 ` [Bug tree-optimization/96722] " jakub at gcc dot gnu.org
2020-08-20 9:39 ` marxin at gcc dot gnu.org
@ 2020-08-20 13:59 ` marxin at gcc dot gnu.org
2020-08-24 11:30 ` jakub at gcc dot gnu.org
` (10 subsequent siblings)
13 siblings, 0 replies; 15+ messages in thread
From: marxin at gcc dot gnu.org @ 2020-08-20 13:59 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96722
Martin Liška <marxin at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Assignee|marxin at gcc dot gnu.org |unassigned at gcc dot gnu.org
Status|ASSIGNED |NEW
--- Comment #2 from Martin Liška <marxin at gcc dot gnu.org> ---
So the problematic transformation happens in einline:
BEFORE:
void foo(S*) (struct S * a)
{
<bb 2> [0.00%] [count: INV]:
if (a_2(D) != 0B)
goto <bb 4>; [0.00%] [count: INV]
else
goto <bb 3>; [0.00%] [count: INV]
<bb 3> [100.00%] [count: INV]:
MEM[(struct &)a_2(D)] ={v} {CLOBBER};
<bb 4> [0.00%] [count: INV]:
return;
}
after:
void foo(S*) (struct S * a)
{
<bb 2> [0.00%] [count: INV]:
if (a_2(D) != 0B)
goto <bb 3>; [0.00%] [count: INV]
else
goto <bb 4>; [0.00%] [count: INV]
<bb 3> [0.00%] [count: INV]:
// predicted unlikely by early return (on trees) predictor.
goto <bb 5>; [0.00%] [count: INV]
<bb 4> [100.00%] [count: INV]:
MEM[(struct &)a_2(D)] ={v} {CLOBBER};
<bb 5> [0.00%] [count: INV]:
return;
}
So as seen, the CFG is more complex as there's an extra GIMPLE_PREDICT.
main then inlines to:
Scope blocks after cleanups:
{ Scope block #0
{ Scope block #7 ../pr96722.C:8 Originating from : extern void S (struct S
*, int);
}
{ Scope block #8 ../pr96722.C:14 Originating from : extern void S (struct S
*, int);
}
}
int main() ()
{
int _5;
<bb 2> [100.00%] [count: INV]:
_5 = 0;
return _5;
}
No longer having address taken: s
Scope blocks after cleanups:
{ Scope block #0
{ Scope block #7 ../pr96722.C:8 Originating from : extern void S (struct S
*, int);
{ Scope block #8 Originating from :#0
}
}
{ Scope block #9 ../pr96722.C:14 Originating from : extern void S (struct S
*, int);
}
}
int main() ()
{
int _5;
<bb 2> [100.00%] [count: INV]:
MEM[(struct &)0B] ={v} {CLOBBER};
_5 = 0;
return _5;
}
So I bet it's hidden in a CFG cleanup where it somehow removes the clobber?
I guess it was latent before my revision..
^ permalink raw reply [flat|nested] 15+ messages in thread* [Bug tree-optimization/96722] [8/9/10/11 Regression] Clobbers on NULL since r8-1519
2020-08-20 9:30 [Bug tree-optimization/96722] New: [8/9/10/11 Regression] Clobbers on NULL since r8-1519 jakub at gcc dot gnu.org
` (2 preceding siblings ...)
2020-08-20 13:59 ` marxin at gcc dot gnu.org
@ 2020-08-24 11:30 ` jakub at gcc dot gnu.org
2020-08-24 12:08 ` rguenther at suse dot de
` (9 subsequent siblings)
13 siblings, 0 replies; 15+ messages in thread
From: jakub at gcc dot gnu.org @ 2020-08-24 11:30 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96722
--- Comment #3 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Created attachment 49111
--> https://gcc.gnu.org/bugzilla/attachment.cgi?id=49111&action=edit
gcc11-pr96722.patch
If *0 ={v} {CLOBBER}; is supposed to be a fancy nop, then we should ignore it
during path isolation (shouldn't infer UB from clobber stmts).
^ permalink raw reply [flat|nested] 15+ messages in thread* [Bug tree-optimization/96722] [8/9/10/11 Regression] Clobbers on NULL since r8-1519
2020-08-20 9:30 [Bug tree-optimization/96722] New: [8/9/10/11 Regression] Clobbers on NULL since r8-1519 jakub at gcc dot gnu.org
` (3 preceding siblings ...)
2020-08-24 11:30 ` jakub at gcc dot gnu.org
@ 2020-08-24 12:08 ` rguenther at suse dot de
2020-08-24 13:07 ` jakub at gcc dot gnu.org
` (8 subsequent siblings)
13 siblings, 0 replies; 15+ messages in thread
From: rguenther at suse dot de @ 2020-08-24 12:08 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96722
--- Comment #4 from rguenther at suse dot de <rguenther at suse dot de> ---
On Mon, 24 Aug 2020, jakub at gcc dot gnu.org wrote:
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96722
>
> --- Comment #3 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
> Created attachment 49111
> --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=49111&action=edit
> gcc11-pr96722.patch
>
> If *0 ={v} {CLOBBER}; is supposed to be a fancy nop, then we should ignore it
> during path isolation (shouldn't infer UB from clobber stmts).
Hmm, I guess the bug is that walk_stmt_load_store_addr_ops considers
clobbers a store. OTOH it may be that callers expect it to do that
(the callback gets passed the stmt to check itself if it wants).
So definitely the patch makes sense but still IMHO CDDCE has a
wrong-code bug here - it makes a conditional clobber unconditional.
Not sure if we can create a wrong-code testcase from C++s placement
of clobbers for a non-NULL case though.
^ permalink raw reply [flat|nested] 15+ messages in thread* [Bug tree-optimization/96722] [8/9/10/11 Regression] Clobbers on NULL since r8-1519
2020-08-20 9:30 [Bug tree-optimization/96722] New: [8/9/10/11 Regression] Clobbers on NULL since r8-1519 jakub at gcc dot gnu.org
` (4 preceding siblings ...)
2020-08-24 12:08 ` rguenther at suse dot de
@ 2020-08-24 13:07 ` jakub at gcc dot gnu.org
2020-08-24 13:49 ` rguenth at gcc dot gnu.org
` (7 subsequent siblings)
13 siblings, 0 replies; 15+ messages in thread
From: jakub at gcc dot gnu.org @ 2020-08-24 13:07 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96722
--- Comment #5 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
On the CDDCE side, I'm afraid I have no idea what can be done.
The pass works by not considering clobber stmts necessary, and when about to
remove (all) clobbers as unnecessary, it instead attempts to keep them unless
they refer to SSA_NAMEs that weren't processed.
At that point, it has no idea if some clobber got turned from conditional into
unconditional, or the more likely case that it is kept as is.
So we'd need to remove (or somehow mark for guaranteed removal) the clobbers
earlier if they were turned from conditional into unconditional.
^ permalink raw reply [flat|nested] 15+ messages in thread* [Bug tree-optimization/96722] [8/9/10/11 Regression] Clobbers on NULL since r8-1519
2020-08-20 9:30 [Bug tree-optimization/96722] New: [8/9/10/11 Regression] Clobbers on NULL since r8-1519 jakub at gcc dot gnu.org
` (5 preceding siblings ...)
2020-08-24 13:07 ` jakub at gcc dot gnu.org
@ 2020-08-24 13:49 ` rguenth at gcc dot gnu.org
2020-08-25 11:50 ` cvs-commit at gcc dot gnu.org
` (6 subsequent siblings)
13 siblings, 0 replies; 15+ messages in thread
From: rguenth at gcc dot gnu.org @ 2020-08-24 13:49 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96722
--- Comment #6 from Richard Biener <rguenth at gcc dot gnu.org> ---
I don't have a good idea either but eventually something along the following -
if we remove any control stmt in a clobbers control dependence chain
force-remove the clobber. Obviously as written it's highly inefficient,
such tracking would need to be done only once per BB. It might also be
too conservative - eventually only the "immediate" control dependence matters
(but the control dependence machinery does not give us this easily).
Eventually we can also check the last_stmt_necessary bitmap instead
of looking at last_stmt itself.
diff --git a/gcc/tree-ssa-dce.c b/gcc/tree-ssa-dce.c
index fae5ae72340..9066dbf6373 100644
--- a/gcc/tree-ssa-dce.c
+++ b/gcc/tree-ssa-dce.c
@@ -1428,6 +1431,23 @@ eliminate_unnecessary_stmts (void)
break;
}
}
+ if (!dead && cd)
+ {
+ bitmap_iterator bi;
+ unsigned edge_number;
+ EXECUTE_IF_SET_IN_BITMAP (cd->get_edges_dependent_on
+ (bb->index),
+ 0, edge_number, bi)
+ {
+ basic_block cd_bb = cd->get_edge_src (edge_number);
+ if (cd_bb != bb
+ && !gimple_plf (last_stmt (cd_bb),
STMT_NECESSARY))
+ {
+ dead = true;
+ break;
+ }
+ }
+ }
if (!dead)
{
bitmap_clear (debug_seen);
@@ -1665,6 +1685,7 @@ tree_dce_done (bool aggressive)
if (aggressive)
{
delete cd;
+ cd = NULL;
sbitmap_free (visited_control_parents);
sbitmap_free (last_stmt_necessary);
sbitmap_free (bb_contains_live_stmts);
^ permalink raw reply [flat|nested] 15+ messages in thread* [Bug tree-optimization/96722] [8/9/10/11 Regression] Clobbers on NULL since r8-1519
2020-08-20 9:30 [Bug tree-optimization/96722] New: [8/9/10/11 Regression] Clobbers on NULL since r8-1519 jakub at gcc dot gnu.org
` (6 preceding siblings ...)
2020-08-24 13:49 ` rguenth at gcc dot gnu.org
@ 2020-08-25 11:50 ` cvs-commit at gcc dot gnu.org
2020-08-25 17:45 ` cvs-commit at gcc dot gnu.org
` (5 subsequent siblings)
13 siblings, 0 replies; 15+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2020-08-25 11:50 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96722
--- 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:a5b15fcb954ba63d58f0daa700281aba33b5f24a
commit r11-2840-ga5b15fcb954ba63d58f0daa700281aba33b5f24a
Author: Jakub Jelinek <jakub@redhat.com>
Date: Tue Aug 25 13:49:40 2020 +0200
gimple: Ignore *0 = {CLOBBER} in path isolation [PR96722]
Clobbers of MEM_REF with NULL address are just fancy nops, something we
just
ignore and don't emit any code for it (ditto for other clobbers), they just
mark end of life on something, so we shouldn't infer from those that there
is some UB.
2020-08-25 Jakub Jelinek <jakub@redhat.com>
PR tree-optimization/96722
* gimple.c (infer_nonnull_range): Formatting fix.
(infer_nonnull_range_by_dereference): Return false for clobber
stmts.
* g++.dg/opt/pr96722.C: New test.
^ permalink raw reply [flat|nested] 15+ messages in thread* [Bug tree-optimization/96722] [8/9/10/11 Regression] Clobbers on NULL since r8-1519
2020-08-20 9:30 [Bug tree-optimization/96722] New: [8/9/10/11 Regression] Clobbers on NULL since r8-1519 jakub at gcc dot gnu.org
` (7 preceding siblings ...)
2020-08-25 11:50 ` cvs-commit at gcc dot gnu.org
@ 2020-08-25 17:45 ` cvs-commit at gcc dot gnu.org
2020-08-25 18:26 ` jakub at gcc dot gnu.org
` (4 subsequent siblings)
13 siblings, 0 replies; 15+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2020-08-25 17:45 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96722
--- Comment #8 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:dff497c29a4f2a4cb3f0cb693e14f30734888845
commit r10-8671-gdff497c29a4f2a4cb3f0cb693e14f30734888845
Author: Jakub Jelinek <jakub@redhat.com>
Date: Tue Aug 25 13:49:40 2020 +0200
gimple: Ignore *0 = {CLOBBER} in path isolation [PR96722]
Clobbers of MEM_REF with NULL address are just fancy nops, something we
just
ignore and don't emit any code for it (ditto for other clobbers), they just
mark end of life on something, so we shouldn't infer from those that there
is some UB.
2020-08-25 Jakub Jelinek <jakub@redhat.com>
PR tree-optimization/96722
* gimple.c (infer_nonnull_range): Formatting fix.
(infer_nonnull_range_by_dereference): Return false for clobber
stmts.
* g++.dg/opt/pr96722.C: New test.
(cherry picked from commit a5b15fcb954ba63d58f0daa700281aba33b5f24a)
^ permalink raw reply [flat|nested] 15+ messages in thread* [Bug tree-optimization/96722] [8/9/10/11 Regression] Clobbers on NULL since r8-1519
2020-08-20 9:30 [Bug tree-optimization/96722] New: [8/9/10/11 Regression] Clobbers on NULL since r8-1519 jakub at gcc dot gnu.org
` (8 preceding siblings ...)
2020-08-25 17:45 ` cvs-commit at gcc dot gnu.org
@ 2020-08-25 18:26 ` jakub at gcc dot gnu.org
2020-09-01 8:33 ` rguenth at gcc dot gnu.org
` (3 subsequent siblings)
13 siblings, 0 replies; 15+ messages in thread
From: jakub at gcc dot gnu.org @ 2020-08-25 18:26 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96722
--- Comment #9 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
The path isolation bug fixed for 10.3+/11+. Keeping open for the DCE issue.
^ permalink raw reply [flat|nested] 15+ messages in thread* [Bug tree-optimization/96722] [8/9/10/11 Regression] Clobbers on NULL since r8-1519
2020-08-20 9:30 [Bug tree-optimization/96722] New: [8/9/10/11 Regression] Clobbers on NULL since r8-1519 jakub at gcc dot gnu.org
` (9 preceding siblings ...)
2020-08-25 18:26 ` jakub at gcc dot gnu.org
@ 2020-09-01 8:33 ` rguenth at gcc dot gnu.org
2020-09-01 10:56 ` [Bug tree-optimization/96722] [8/9 " jakub at gcc dot gnu.org
` (2 subsequent siblings)
13 siblings, 0 replies; 15+ messages in thread
From: rguenth at gcc dot gnu.org @ 2020-09-01 8:33 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96722
--- Comment #10 from Richard Biener <rguenth at gcc dot gnu.org> ---
(In reply to Jakub Jelinek from comment #9)
> The path isolation bug fixed for 10.3+/11+. Keeping open for the DCE issue.
Hmm, for easier tracking I'd use a separate bug for this.
^ permalink raw reply [flat|nested] 15+ messages in thread* [Bug tree-optimization/96722] [8/9 Regression] Clobbers on NULL since r8-1519
2020-08-20 9:30 [Bug tree-optimization/96722] New: [8/9/10/11 Regression] Clobbers on NULL since r8-1519 jakub at gcc dot gnu.org
` (10 preceding siblings ...)
2020-09-01 8:33 ` rguenth at gcc dot gnu.org
@ 2020-09-01 10:56 ` jakub at gcc dot gnu.org
2020-09-16 19:23 ` cvs-commit at gcc dot gnu.org
2020-09-17 17:58 ` jakub at gcc dot gnu.org
13 siblings, 0 replies; 15+ messages in thread
From: jakub at gcc dot gnu.org @ 2020-09-01 10:56 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96722
Jakub Jelinek <jakub at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Summary|[8/9/10/11 Regression] |[8/9 Regression] Clobbers
|Clobbers on NULL since |on NULL since r8-1519
|r8-1519 |
--- Comment #11 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
CDDCE part moved to PR96881. This bug tracks the path isolation, which is now
fixed for 10.3+ and 11.1+.
^ permalink raw reply [flat|nested] 15+ messages in thread* [Bug tree-optimization/96722] [8/9 Regression] Clobbers on NULL since r8-1519
2020-08-20 9:30 [Bug tree-optimization/96722] New: [8/9/10/11 Regression] Clobbers on NULL since r8-1519 jakub at gcc dot gnu.org
` (11 preceding siblings ...)
2020-09-01 10:56 ` [Bug tree-optimization/96722] [8/9 " jakub at gcc dot gnu.org
@ 2020-09-16 19:23 ` cvs-commit at gcc dot gnu.org
2020-09-17 17:58 ` jakub at gcc dot gnu.org
13 siblings, 0 replies; 15+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2020-09-16 19:23 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96722
--- Comment #12 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-9 branch has been updated by Jakub Jelinek
<jakub@gcc.gnu.org>:
https://gcc.gnu.org/g:acd97038dba34830520ed58915d509d21348b554
commit r9-8910-gacd97038dba34830520ed58915d509d21348b554
Author: Jakub Jelinek <jakub@redhat.com>
Date: Tue Aug 25 13:49:40 2020 +0200
gimple: Ignore *0 = {CLOBBER} in path isolation [PR96722]
Clobbers of MEM_REF with NULL address are just fancy nops, something we
just
ignore and don't emit any code for it (ditto for other clobbers), they just
mark end of life on something, so we shouldn't infer from those that there
is some UB.
2020-08-25 Jakub Jelinek <jakub@redhat.com>
PR tree-optimization/96722
* gimple.c (infer_nonnull_range): Formatting fix.
(infer_nonnull_range_by_dereference): Return false for clobber
stmts.
* g++.dg/opt/pr96722.C: New test.
(cherry picked from commit a5b15fcb954ba63d58f0daa700281aba33b5f24a)
^ permalink raw reply [flat|nested] 15+ messages in thread* [Bug tree-optimization/96722] [8/9 Regression] Clobbers on NULL since r8-1519
2020-08-20 9:30 [Bug tree-optimization/96722] New: [8/9/10/11 Regression] Clobbers on NULL since r8-1519 jakub at gcc dot gnu.org
` (12 preceding siblings ...)
2020-09-16 19:23 ` cvs-commit at gcc dot gnu.org
@ 2020-09-17 17:58 ` jakub at gcc dot gnu.org
13 siblings, 0 replies; 15+ messages in thread
From: jakub at gcc dot gnu.org @ 2020-09-17 17:58 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96722
Jakub Jelinek <jakub at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Resolution|--- |FIXED
Status|NEW |RESOLVED
--- Comment #13 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Fixed for 8.5 in r8-10513-g5378a7496db299c06f3be72a2db2eab280f29dbb and for
9.4+ by the above commit.
^ permalink raw reply [flat|nested] 15+ messages in thread