| Message ID | bf92e97863c5b6ed50f4f12738c2a11609398adf.1447037918.git.segher@kernel.crashing.org |
|---|---|
| State | New |
| Headers | show |
On 11/08/2015 08:09 PM, Segher Boessenkool wrote: > The code mistakenly thinks any cond_jump has two successors. This is > not true if both destinations are the same, as can happen with weird > patterns as in the PR. > > Bootstrapped and tested on powerpc64-linux; also tested the simplified > test in the PR on an x86_64-linux cross. > > Sorry for the breakage. Is this okay for trunk? > > > Segher > > > 2015-11-09 Segher Boessenkool <segher@kernel.crashing.org> > > * gcc/bb-reorder.c (reorder_basic_blocks_simple): Treat a conditional > branch with only one successor just like unconditional branches. OK. Though this begs the question, should something have cleaned that up prior to bb-reorder? Don't forget the PR marker in the committed ChangeLog. jeff
On Sun, Nov 08, 2015 at 08:21:47PM -0700, Jeff Law wrote: > On 11/08/2015 08:09 PM, Segher Boessenkool wrote: > >The code mistakenly thinks any cond_jump has two successors. This is > >not true if both destinations are the same, as can happen with weird > >patterns as in the PR. > > > >Bootstrapped and tested on powerpc64-linux; also tested the simplified > >test in the PR on an x86_64-linux cross. > > > >Sorry for the breakage. Is this okay for trunk? > > > > > >Segher > > > > > >2015-11-09 Segher Boessenkool <segher@kernel.crashing.org> > > > > * gcc/bb-reorder.c (reorder_basic_blocks_simple): Treat a conditional > > branch with only one successor just like unconditional branches. > OK. Though this begs the question, should something have cleaned that > up prior to bb-reorder? It normally does (which I why I hadn't noticed it), but there is no unconditional version of this in the machine description. It seems to create a conditional branch so that it can do a move from a pseudo (that it sets in the branch pattern itself, it's a parallel) to the AX register. bb-reorder runs after RA so that move has been folded, the pseudo itself is in AX already, so both arms of the conditional now point to the next insn. I don't know why the backend cannot put AX directly in this pattern (or while expanding it). Either way, bb-reorder should be able to handle the situation. > Don't forget the PR marker in the committed ChangeLog. Uh yes, thanks. Segher
diff --git a/gcc/bb-reorder.c b/gcc/bb-reorder.c index 5f1c2cc..950b1a1 100644 --- a/gcc/bb-reorder.c +++ b/gcc/bb-reorder.c @@ -2304,7 +2304,9 @@ reorder_basic_blocks_simple (void) if (JUMP_P (end) && extract_asm_operands (end)) continue; - if (any_condjump_p (end)) + if (single_succ_p (bb)) + edges[n++] = EDGE_SUCC (bb, 0); + else if (any_condjump_p (end)) { edge e0 = EDGE_SUCC (bb, 0); edge e1 = EDGE_SUCC (bb, 1); @@ -2315,8 +2317,6 @@ reorder_basic_blocks_simple (void) edges[n++] = e0; edges[n++] = e1; } - else if (single_succ_p (bb)) - edges[n++] = EDGE_SUCC (bb, 0); } /* Sort the edges, the most desirable first. When optimizing for size