| Message ID | 52A6B510.9060607@redhat.com |
|---|---|
| State | New |
| Headers | show |
On Tue, Dec 10, 2013 at 7:30 AM, Jeff Law wrote: > > > Ping! I'd prefer going with the first patch to merge_if_blocks (with a big "???" or FIXME), to contain this problem to where it occurs. Otherwise, before you know it, there are other places in the compiler that assume it's OK to merge blocks that are not can_merge_block_p. Ciao! Steven
On 12/10/13 16:22, Steven Bosscher wrote: > On Tue, Dec 10, 2013 at 7:30 AM, Jeff Law wrote: >> >> >> Ping! > > I'd prefer going with the first patch to merge_if_blocks (with a big > "???" or FIXME), to contain this problem to where it occurs. > Otherwise, before you know it, there are other places in the compiler > that assume it's OK to merge blocks that are not can_merge_block_p. I think the patch as posted in the RFA is better based on the robustness principle. Nothing stops an RTL pass from asking for two blocks to be merged when there's a BARRIER between them. So rtl_merge_blocks should do the right thing when presented with that situation. Furthermore, I do not believe that can_merge_blocks_p should reject merging of blocks where there's a BARRIER between them. There is a very legitimate use for that that arises from conditional execution and from the use of __builtin_unreachable. Obviously if you succeed in removing BARRIERS after 4.9, then all this becomes moot. But as long as BARRIERS exist, we should handle them properly. Jeff
diff --git a/gcc/cfgrtl.c b/gcc/cfgrtl.c index 63f44af..99ebe85 100644 --- a/gcc/cfgrtl.c +++ b/gcc/cfgrtl.c @@ -878,8 +878,15 @@ rtl_merge_blocks (basic_block a, basic_block b) a_end = PREV_INSN (del_first); } - else if (BARRIER_P (NEXT_INSN (a_end))) - del_first = NEXT_INSN (a_end); + + /* If there is a BARRIER between A & B, remove it. This can happen + if a block with no successors is if-converted. */ + rtx end = NEXT_INSN (a_end); + while (end && NOTE_P (end) && !NOTE_INSN_BASIC_BLOCK_P (end)) + end = NEXT_INSN (end); + + if (BARRIER_P (end)) + del_first = end; /* Delete everything marked above as well as crap that might be hanging out between the two blocks. */
Ping! -------- Original Message -------- Subject: [PATCH][RFA][PR middle-end/59285] Handle BARRIERS between blocks in rtl_merge_blocks Date: Thu, 05 Dec 2013 21:52:33 -0700 From: Jeff Law <law@redhat.com> To: gcc-patches <gcc-patches@gcc.gnu.org> builtin-unreachable-6.c when compiled for armv7l has a conditional where the fall-thru block has no successors (due to __builtin_unreachable) and is immediately followed by the jump-to block. In this situation there will be a BARRIER after the fall-thru block (remember, it has no successors). The post register allocation if-conversion pass wants to if-convert the whole mess (which is valid and often profitable). The result should be a single block with any insns from the THEN/ELSE blocks being suitably predicated. This all works pretty much as expected, except the BARRIER is still in the insn chain, which causes a checking failure after if-conversion is complete. The problem is the if-conversion code is calling merge_blocks, which in turn calls rtl_merge_blocks to merge the test block, else block and then block. rtl_merge_blocks does not properly detect and eliminate any barriers that may be between the blocks to be merged. Long term BARRIERS should just go away, but that's not stage3 material. Bootstrapped and regression tested in progress on armv7l-unknown-linux-gnueabihf. Fixes builtin-unreachable-6, of course :-) OK for the trunk? Thanks, Jeff * cfgrtl.c (rtl_merge_blocks): Properly search for and remove any BARRIER between blocks A & B.