| Message ID | 3bfab45a-2a04-e834-8af6-3390c8147347@mentor.com |
|---|---|
| State | New |
| Headers | show |
| Series | [tail-merge,PR84956] Don't merge bbs with bb_has_abnormal_pred | expand |
On Thu, 22 Mar 2018, Tom de Vries wrote: > Hi, > > This fixes a 6/7/8 regression, a P3 ICE in the tail-merge pass. > > > Consider the test-case from the patch. > > When compiling at -O2, duplicates are found in tail-merge: > ... > find_duplicates: <bb 6> duplicate of <bb 7> > find_duplicates: <bb 6> duplicate of <bb 9> > ... > So, tail-merge calls replace_block_by (bb7, bb6). > > However, bb7 has an abnormal edge bb5 -> bb7 as predecessor (bb6 has an > abnormal edge as predecessor as well, but that doesn't look necessary to > trigger the ICE): > ... > ;; basic block 9, loop depth 0 > ;; prev block 3, next block 4, flags: (NEW, REACHABLE) > ;; pred: 3 [50.0% (guessed)] (FALSE_VALUE,EXECUTABLE) > goto <bb 8>; [100.00%] > ;; succ: 8 [always] (FALLTHRU,EXECUTABLE) > > ;; basic block 6, loop depth 0 > ;; prev block 5, next block 7, flags: (NEW, REACHABLE, VISITED) > ;; pred: 5 [33.3% (guessed)] (ABNORMAL,EXECUTABLE) > goto <bb 8>; [100.00%] > ;; succ: 8 [always] (FALLTHRU,EXECUTABLE) > > ;; basic block 7, loop depth 0 > ;; prev block 6, next block 8, flags: (NEW, REACHABLE, VISITED) > ;; pred: 5 [33.3% (guessed)] (ABNORMAL,EXECUTABLE) > ;; succ: 8 [always] (FALLTHRU,EXECUTABLE) > ... > > So when replace_block_by calls redirect_edge_and_branch (bb5 -> bb7, bb6) it > lands in gimple_redirect_edge_and_branch, and fails here: > ... > 6024 if (e->flags & EDGE_ABNORMAL) > 6025 return NULL; > ... > > which causes this assert to trigger: > ... > gcc_assert (pred_edge != NULL); > ... > > > The patch fixes the ICE conservatively by skipping bbs with > bb_has_abnormal_preds in find_clusters_1. > > [ A more optimal fix could be to detect in this example that there's an > abnormal edge bb5 -> bb6, and skip redirecting bb5 -> bb7 to bb6. ] > > Bootstrapped and reg-tested on x86_64. > > OK for stage4, gcc-6-branch and gcc-7-branch? OK everywhere. Thanks, Richard.
[tail-merge] Don't merge bbs with bb_has_abnormal_pred 2018-03-21 Tom de Vries <tom@codesourcery.com> PR tree-optimization/84956 * tree-ssa-tail-merge.c (find_clusters_1): Skip bbs with bb_has_abnormal_pred. * gcc.dg/pr84956.c: New test. --- gcc/testsuite/gcc.dg/pr84956.c | 27 +++++++++++++++++++++++++++ gcc/tree-ssa-tail-merge.c | 6 ++++-- 2 files changed, 31 insertions(+), 2 deletions(-) diff --git a/gcc/testsuite/gcc.dg/pr84956.c b/gcc/testsuite/gcc.dg/pr84956.c new file mode 100644 index 0000000..055a749 --- /dev/null +++ b/gcc/testsuite/gcc.dg/pr84956.c @@ -0,0 +1,27 @@ +/* { dg-options "-O2 -ftree-tail-merge" } */ + +char a; +int c; +unsigned b (); + +unsigned +setjmp () +{ +} + +static void +d () +{ + if (b ()) + c = 3; +} + +void +e () +{ + d (); + a && ({ setjmp (); }); + a && ({ setjmp (); }); + a && ({ setjmp (); }); +} + diff --git a/gcc/tree-ssa-tail-merge.c b/gcc/tree-ssa-tail-merge.c index a687c3f..f482ce1 100644 --- a/gcc/tree-ssa-tail-merge.c +++ b/gcc/tree-ssa-tail-merge.c @@ -1455,7 +1455,8 @@ find_clusters_1 (same_succ *same_succ) /* TODO: handle blocks with phi-nodes. We'll have to find corresponding phi-nodes in bb1 and bb2, with the same alternatives for the same preds. */ - if (bb_has_non_vop_phi (bb1) || bb_has_eh_pred (bb1)) + if (bb_has_non_vop_phi (bb1) || bb_has_eh_pred (bb1) + || bb_has_abnormal_pred (bb1)) continue; nr_comparisons = 0; @@ -1463,7 +1464,8 @@ find_clusters_1 (same_succ *same_succ) { bb2 = BASIC_BLOCK_FOR_FN (cfun, j); - if (bb_has_non_vop_phi (bb2) || bb_has_eh_pred (bb2)) + if (bb_has_non_vop_phi (bb2) || bb_has_eh_pred (bb2) + || bb_has_abnormal_pred (bb2)) continue; if (BB_CLUSTER (bb1) != NULL && BB_CLUSTER (bb1) == BB_CLUSTER (bb2))
Hi, This fixes a 6/7/8 regression, a P3 ICE in the tail-merge pass. Consider the test-case from the patch. When compiling at -O2, duplicates are found in tail-merge: ... find_duplicates: <bb 6> duplicate of <bb 7> find_duplicates: <bb 6> duplicate of <bb 9> ... So, tail-merge calls replace_block_by (bb7, bb6). However, bb7 has an abnormal edge bb5 -> bb7 as predecessor (bb6 has an abnormal edge as predecessor as well, but that doesn't look necessary to trigger the ICE): ... ;; basic block 9, loop depth 0 ;; prev block 3, next block 4, flags: (NEW, REACHABLE) ;; pred: 3 [50.0% (guessed)] (FALSE_VALUE,EXECUTABLE) goto <bb 8>; [100.00%] ;; succ: 8 [always] (FALLTHRU,EXECUTABLE) ;; basic block 6, loop depth 0 ;; prev block 5, next block 7, flags: (NEW, REACHABLE, VISITED) ;; pred: 5 [33.3% (guessed)] (ABNORMAL,EXECUTABLE) goto <bb 8>; [100.00%] ;; succ: 8 [always] (FALLTHRU,EXECUTABLE) ;; basic block 7, loop depth 0 ;; prev block 6, next block 8, flags: (NEW, REACHABLE, VISITED) ;; pred: 5 [33.3% (guessed)] (ABNORMAL,EXECUTABLE) ;; succ: 8 [always] (FALLTHRU,EXECUTABLE) ... So when replace_block_by calls redirect_edge_and_branch (bb5 -> bb7, bb6) it lands in gimple_redirect_edge_and_branch, and fails here: ... 6024 if (e->flags & EDGE_ABNORMAL) 6025 return NULL; ... which causes this assert to trigger: ... gcc_assert (pred_edge != NULL); ... The patch fixes the ICE conservatively by skipping bbs with bb_has_abnormal_preds in find_clusters_1. [ A more optimal fix could be to detect in this example that there's an abnormal edge bb5 -> bb6, and skip redirecting bb5 -> bb7 to bb6. ] Bootstrapped and reg-tested on x86_64. OK for stage4, gcc-6-branch and gcc-7-branch? Thanks, - Tom