Summary: | [4.8 Regression] 255.vortex in SPEC CPU 2000 failed to build | ||
---|---|---|---|
Product: | gcc | Reporter: | H.J. Lu <hjl.tools> |
Component: | middle-end | Assignee: | Richard Biener <rguenth> |
Status: | RESOLVED FIXED | ||
Severity: | normal | CC: | areg.melikadamyan, izamyatin, jeffreyalaw, law |
Priority: | P3 | ||
Version: | 4.8.0 | ||
Target Milestone: | 4.8.0 | ||
Host: | Target: | ||
Build: | Known to work: | ||
Known to fail: | Last reconfirmed: | 2012-03-29 00:00:00 | |
Bug Depends on: | |||
Bug Blocks: | 52808 |
Description
H.J. Lu
2012-03-28 17:31:50 UTC
Confirmed. Mine. Reduced testcase, but a different ICE than the original one. void Env_FetchObj0AttrOffset (unsigned int NumFields, int *Status) { int Found = 0; if (NumFields) while ((*Status == 0) && NumFields-- > 0 && Found == 0) Found = 1; } Reduced testcase for the original ICE: void Env_FetchObj0AttrOffset (unsigned int NumFields, int AttrNum, int *Status, int *Vfields) { int Found = 0; if (NumFields) while ((*Status == 0) && NumFields-- > 0 && Found == 0) if (*Vfields == AttrNum) Found = 1; } DOM jump threading threads the loop latch edge: if (latch->aux) { /* First handle the case latch edge is redirected. */ loop->latch = thread_single_edge (latch); gcc_assert (single_succ (loop->latch) == tgt_bb); loop->header = tgt_bb; /* Thread the remaining edges through the former header. */ thread_block (header, false); but the updated loop->latch, loop->header pair is certainly not the proper one. That's the real bug. Jump threading rotates the loop here but fails to realize that - the new header needs to dominate all of the loops remaining BBs, and determine_bb_domination_status does not verify that (either by design or because it is buggy). The symptom is then caused by cfgcleaup merging latch and header and simple latch creation being confused. (In reply to comment #3) > DOM jump threading threads the loop latch edge: > > if (latch->aux) > { > /* First handle the case latch edge is redirected. */ > loop->latch = thread_single_edge (latch); > gcc_assert (single_succ (loop->latch) == tgt_bb); > loop->header = tgt_bb; At this point everything is still ok, but > /* Thread the remaining edges through the former header. */ > thread_block (header, false); this creates a multiple entry loop (with the old entry being an unreachable block after threading - sth cfgcleaup exposes, which in turn would turn that into a valid loop, albeit with bogus header/latch, again). Now, thread_through_loop_header ensures (fingers crossing) that we never create a multiple entry loop. Thus the following fix would work. Index: gcc/tree-ssa-threadupdate.c =================================================================== --- gcc/tree-ssa-threadupdate.c (revision 186007) +++ gcc/tree-ssa-threadupdate.c (working copy) @@ -649,6 +649,17 @@ thread_block (basic_block bb, bool noloo || THREAD_TARGET2 (e)))) continue; + /* thread_through_loop_header made sure we are not creating + a multiple entry loop. If we are creating a new loop + entry the destination will become the new header of the + loop and the old entry will become unreachable. */ + if (e->src->loop_father != e2->dest->loop_father) + { + e2->dest->loop_father->header = e2->dest; + /* Discard knowledge about the latch. */ + e2->dest->loop_father->latch = NULL; + } + if (e->dest == e2->src) update_bb_profile_for_threading (e->dest, EDGE_FREQUENCY (e), e->count, THREAD_TARGET (e)); Zdenek certainly knows this part of the code better than I and may comment, but it looks reasonable to me. It doesn't work, as we expect loop_latch_edge () to work during further threading. It also does not work because we miss some threadings and thus hit the assert that e->aux is NULL after threading is done. The issue is that we thread both the latch and the loop entry edge. But the code special casing each case does not consider that both may happen at the same time. If we'd re-start at thread_through_loop_header for the case in this bug we'd fail, as the entry edges THREAD_TARGET2 is set. Thus, I'll try again with Index: gcc/tree-ssa-threadupdate.c =================================================================== --- gcc/tree-ssa-threadupdate.c (revision 186066) +++ gcc/tree-ssa-threadupdate.c (working copy) @@ -838,6 +838,7 @@ thread_through_loop_header (struct loop edge_iterator ei; basic_block tgt_bb, atgt_bb; enum bb_dom_status domst; + bool threaded_latch = false; /* We have already threaded through headers to exits, so all the threading requests now are to the inside of the loop. We need to avoid creating @@ -908,6 +909,7 @@ thread_through_loop_header (struct loop if (single_succ_p (header)) goto fail; +thread_rest: if (latch->aux) { if (THREAD_TARGET2 (latch)) @@ -916,7 +918,7 @@ thread_through_loop_header (struct loop tgt_bb = tgt_edge->dest; } else if (!may_peel_loop_headers - && !redirection_block_p (loop->header)) + && !redirection_block_p (header)) goto fail; else { @@ -950,7 +952,7 @@ thread_through_loop_header (struct loop if (!tgt_bb) { /* There are no threading requests. */ - return false; + return threaded_latch; } /* Redirecting to empty loop latch is useless. */ @@ -971,7 +973,7 @@ thread_through_loop_header (struct loop loop->header = NULL; loop->latch = NULL; loops_state_set (LOOPS_NEED_FIXUP); - return thread_block (header, false); + return threaded_latch | thread_block (header, false); } if (tgt_bb->loop_father->header == tgt_bb) @@ -994,9 +996,10 @@ thread_through_loop_header (struct loop loop->latch = thread_single_edge (latch); gcc_assert (single_succ (loop->latch) == tgt_bb); loop->header = tgt_bb; + threaded_latch = true; /* Thread the remaining edges through the former header. */ - thread_block (header, false); + goto thread_rest; } else { @@ -1039,7 +1042,7 @@ fail: free (e->aux); e->aux = NULL; } - return false; + return threaded_latch; } /* Walk through the registered jump threads and convert them into a With that patch gcc.dg/tree-ssa/ssa-dom-thread-2.c regresses as we do not perform the threading through the header edge anymore. Note that using thread_block (header, true); for the rest of the header edges does not work either, as it still happily creates the multiple entry loop with the bogus header. The following is another possibility - remove the remaining threadings that would be harmful but allow those that would be safe. Index: gcc/tree-ssa-threadupdate.c =================================================================== --- gcc/tree-ssa-threadupdate.c (revision 186082) +++ gcc/tree-ssa-threadupdate.c (working copy) @@ -826,6 +826,17 @@ determine_bb_domination_status (struct l return (bb_reachable ? DOMST_DOMINATING : DOMST_LOOP_BROKEN); } +/* Return true if BB is part of the new pre-header that is created + when threading the latch to DATA. */ + +static bool +def_split_header_continue_p (const_basic_block bb, const void *data) +{ + const_basic_block new_header = (const_basic_block) data; + return (bb->loop_father == new_header->loop_father + && bb != new_header); +} + /* Thread jumps through the header of LOOP. Returns true if cfg changes. If MAY_PEEL_LOOP_HEADERS is false, we avoid threading from entry edges to the inside of the loop. */ @@ -990,11 +1001,46 @@ thread_through_loop_header (struct loop if (latch->aux) { + basic_block *bblocks; + unsigned nblocks, i; + /* First handle the case latch edge is redirected. */ loop->latch = thread_single_edge (latch); gcc_assert (single_succ (loop->latch) == tgt_bb); loop->header = tgt_bb; + /* Remove the new pre-header blocks from our loop. */ + bblocks = XCNEWVEC (basic_block, loop->num_nodes); + nblocks = dfs_enumerate_from (header, 0, def_split_header_continue_p, + bblocks, loop->num_nodes, tgt_bb); + for (i = 0; i < nblocks; i++) + { + remove_bb_from_loops (bblocks[i]); + add_bb_to_loop (bblocks[i], loop_outer (loop)); + } + free (bblocks); + + /* Cancel remaining threading requests that would make the + loop a multiple entry loop. */ + FOR_EACH_EDGE (e, ei, header->preds) + { + edge e2; + if (e->aux == NULL) + continue; + + if (THREAD_TARGET2 (e)) + e2 = THREAD_TARGET2 (e); + else + e2 = THREAD_TARGET (e); + + if (e->src->loop_father != e2->dest->loop_father + && e2->dest != loop->header) + { + free (e->aux); + e->aux = NULL; + } + } + /* Thread the remaining edges through the former header. */ thread_block (header, false); } Author: rguenth Date: Mon Apr 2 15:13:45 2012 New Revision: 186085 URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=186085 Log: 2012-04-02 Richard Guenther <rguenther@suse.de> PR tree-optimization/52756 * tree-ssa-threadupdate.c (def_split_header_continue_p): New function. (thread_through_loop_header): After threading through the loop latch remove the split part from the loop and clear further threading opportunities that would create a multiple entry loop. * gcc.dg/torture/pr52756.c: New testcase. Added: trunk/gcc/testsuite/gcc.dg/torture/pr52756.c Modified: trunk/gcc/ChangeLog trunk/gcc/testsuite/ChangeLog trunk/gcc/tree-ssa-threadupdate.c Fixed. |