RFR: 8267532: C2: Profile and prune untaken exception handlers [v5]

[Jorn Vernee]

On Wed, 15 Nov 2023 03:45:48 GMT, Vladimir Ivanov <vlivanov at openjdk.org> wrote:

> > > I don't fully understand the issue with has_monitor. It does look like a pre-existing issue and it's better to handle it separately.
> 
> > I don't mind moving it to a separate patch, but I don't think it's possible to trigger a failure of the current code without the changes in this patch. So, I don't think it would be possible to add a test in that case.
> 
> As of now, C2 should prune exception handlers when the corresponding exception class is unloaded. How is it different from what you implemented?

The issue occurs for OSR compilations where the monitorenter is before the loop (outside of the compiled code), and all the monitorexits are pruned. In that case the existing logic does not detect that the method has monitors during parsing. javac generates 2 monitorexists or synchronized blocks/method: one for the normal exit, and one in a synthetic exception handler. For OSR compilations, the regular monitorexit gets pruned because it is in a dead branch (after the loop). Theoretically if the handler used an unloaded exception type, it would be pruned as well in the existing code. But, javac currently only generates an exception handler that catches `any` exception type, as far as I can tell. 

Though, thinking about it, I can try generating some bytecode that uses a different handler with a custom (unloaded) exception type instead, and see if I can get both the `monitorexits` to be pruned.

> Irrespective of where it is placed (beginning of a block, invoke bytecode, or any other bytecode instruction), an uncommon trap effectively prunes everything it dominates.

Okay, I see what you mean. Then my argument is not valid.

> Still, IMO enhancing profiling machinery to track unreached exception handlers is a good thing.

> In general, pruning effectively unreachable calls looks like the more in the right direction. But it looks riskier and, hence, requires more performance validation.

So, how do you want to move forward? Should I attempt to re-implement the current patch to prune infrequent calls instead?

-------------

PR Comment: https://git.openjdk.org/jdk/pull/16416#issuecomment-1811776416
PR Comment: https://git.openjdk.org/jdk/pull/16416#issuecomment-1811777862