| Message ID | Pine.LNX.4.64.1201251702250.16449@wotan.suse.de |
|---|---|
| State | New |
| Headers | show |
On 01/26/2012 03:04 AM, Michael Matz wrote: > Actually, resx/eh_dispatch always are the last BB statements, so the loop > doesn't need to look at all statements in a BB, making it quite somewhat > faster. Consider the tree-eh.c to be looking like so: For the record, is this without optimization or something? A region without a landing pad is unreachable. Thus I'd normally expect the region's blocks, including the offending RESX referencing the region, to be deleted as dead code. Otherwise, this second patch is ok. r~
On Wed, Jan 25, 2012 at 10:54 PM, Richard Henderson <rth@redhat.com> wrote: > On 01/26/2012 03:04 AM, Michael Matz wrote: >> Actually, resx/eh_dispatch always are the last BB statements, so the loop >> doesn't need to look at all statements in a BB, making it quite somewhat >> faster. Consider the tree-eh.c to be looking like so: > > For the record, is this without optimization or something? > > A region without a landing pad is unreachable. Thus I'd normally > expect the region's blocks, including the offending RESX referencing > the region, to be deleted as dead code. Even without optimization I hope. > Otherwise, this second patch is ok. > > > r~
Hi, On Thu, 26 Jan 2012, Richard Henderson wrote: > On 01/26/2012 03:04 AM, Michael Matz wrote: > > Actually, resx/eh_dispatch always are the last BB statements, so the loop > > doesn't need to look at all statements in a BB, making it quite somewhat > > faster. Consider the tree-eh.c to be looking like so: > > For the record, is this without optimization or something? Yes, it's with various -fno-xyz options that result in certain code to be removed later than usual. But it seems to me that with a sophisticated enough testcase it could be reproduced also with just -O2, because ... > A region without a landing pad is unreachable. Thus I'd normally expect > the region's blocks, including the offending RESX referencing the > region, to be deleted as dead code. ... just because a region has no landing pads anymore merely means any associated RESX is externally throwing. The resx itself might very well still be reachable. That's what happens with that testcase, the situation before the first ehcleanup: 1 cleanup land:{2,<L23>} 2 cleanup land:{1,<L21>} 3 must_not_throw ... finally_tmp.4D.1912_6 = 1; switch (finally_tmp.4D.1912_6) <default: <L22>, case 1: <L24>> ... <L24>: return; # SUCC: EXIT # BLOCK 4 # PRED: 2 <L22>: [LP 2] resx 2 # SUCC: 5 (eh) # BLOCK 5 # PRED: 4 (eh) <L23>: [LP 2] resx 1 # SUCC: Nothing is using landing pad 1 (L21, indeed that label isn't in the insn stream anymore even). So we remove bb 5 and mark the resx2 as not be associated with a region anymore, leaving us with: 1 cleanup land:{2,<L23>} 2 cleanup ... finally_tmp.4D.1912_6 = 1; switch (finally_tmp.4D.1912_6) <default: <L22>, case 1: <L24>> ... <L24>: return; # SUCC: EXIT # BLOCK 4 # PRED: 2 <L22>: resx 2 # SUCC: Note how bb 4 can still be reached by the switch (that will later be optimized, because in reality finally_tmp.4D.1912_6 will always be 1, therefore that path is unreachable), and will be externally throwing. Without the patch, though, we now happily remove region 2, without doing anything on that resx (and I'm not sure we actually can do much about it, it must stay something externally throwing). And the removed section 2 then wreaks havok in the inliner when it wants to remap the resx statement. Perhaps the better solution would simply be to robustify the inliner (a resx with a null region is externally throwing and hence, when inlined could be transformed into a resx associated with the innermost region of the inline call) instead of avoiding to remove the region, but as Jakub already went the current way with remove_unreachable_handlers I thought it most conservative for this stage to do similar. > Otherwise, this second patch is ok. Thanks. r183559. Ciao, Michael.
Index: tree-eh.c =================================================================== --- tree-eh.c (revision 183524) +++ tree-eh.c (working copy) @@ -3617,14 +3617,40 @@ remove_unreachable_handlers_no_lp (void) { eh_region r; int i; + sbitmap r_reachable; + basic_block bb; + + r_reachable = sbitmap_alloc (VEC_length (eh_region, cfun->eh->region_array)); + sbitmap_zero (r_reachable); + + FOR_EACH_BB (bb) + { + gimple stmt = last_stmt (bb); + if (stmt) + /* Avoid removing regions referenced from RESX/EH_DISPATCH. */ + switch (gimple_code (stmt)) + { + case GIMPLE_RESX: + SET_BIT (r_reachable, gimple_resx_region (stmt)); + break; + case GIMPLE_EH_DISPATCH: + SET_BIT (r_reachable, gimple_eh_dispatch_region (stmt)); + break; + default: + break; + } + } for (i = 1; VEC_iterate (eh_region, cfun->eh->region_array, i, r); ++i) - if (r && r->landing_pads == NULL && r->type != ERT_MUST_NOT_THROW) + if (r && r->landing_pads == NULL && r->type != ERT_MUST_NOT_THROW + && !TEST_BIT (r_reachable, i)) { if (dump_file) fprintf (dump_file, "Removing unreachable region %d\n", i); remove_eh_handler (r); } + + sbitmap_free (r_reachable); } /* Undo critical edge splitting on an EH landing pad. Earlier, we