Re: Fix PR48794 some more

[Michael Matz <matz at suse dot de>]

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.