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

[Jorn Vernee]

On Tue, 14 Nov 2023 03:47:50 GMT, Vladimir Ivanov <vlivanov at openjdk.org> wrote:

> What kind of performance testing have you done?

I tested with the benchmarks attached to the PR. All of those improve.

I also did some general benchmark runs like DaCapo, specjbb2008, and other internal startup and compiler benchmarks we have (not that Renaisance is broken on the latest JDK). I didn't spot any big differences, but I'm currently doing more followup testing to eliminate false positives/negatives.
 
> The current bug summary is too vague. Please, reword it describing what the proposed enhancement does.

Done.

> 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.

> It's interesting to note that the underlying issue for FFM is not that exception handlers aren't profiled, but that unreached calls are not pruned. It complicates the job for EA making arguments non-scalarizable. Pruning unreachable calls would fix the issue in a more disciplined manner, but it would also have more pervasive effects requiring deeper performance evaluation. Overall, it would be helpful to ensure there are no unreachable calls encountered during C2 compilation at all.

My sense when working on this was that C2 relies on dead branches being pruned to eliminate unreached calls (and other code) within those branches. That potentially allows multiple unreached calls to be eliminated using a single uncommon trap for the whole branch. It also allows eliminating other dead code for which we don't profile. So, in other words: pruning at the branch level seems more efficient. Exception dispatch/handlers are just one type of 'branch' that we don't handle at the moment.

I agree it would be useful to do a followup search for other cases in which we encounter unreached calls, as an indicator that we failed to eliminate a dead branch. One case I can think of where a block as a whole might appear 'alive', while a call somewhere in that block appears 'dead', is when an instruction before the call always throws an exception, meaning the branch is taken, but the call never executes. Are there other cases you had in mind?
 
> `ciTypeFlow` may benefit from new profiling information as well.

Do you mean that successors of an untaken exception handler do not need to care about the type updates in the exception handler block? I can file an RFE if you want.
 
>     * I don't see much value in 2 separate product flags to control profiling and optimization logic (`ProfileExceptionHandlers` and `PruneDeadExceptionHandlers`); having a single product flag should be enough;

Okay, I can use `PruneDeadExceptionHandlers` everywhere instead. Or do you want to just always turn on the profiling?

>     * product flags should be marked diagnostic

Ok.

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

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