Use profilingOpts strategy under JitProfiling mode only

[hzongaro]

The profilingOpts optimization strategy includes profileGenerator and deadTreesElimination. The profileGenerator simply returns if the JProfiling mode is enabled, while the deadTreesElimination is intended to clean up trees that might have been introduced by profileGenerator, so it too is really only needed if JitProfilng mode is enabled. However, the optimization strategy as a whole is being run if either JProfiling or JitProfiling is enabled.

Some optimization strategies run compactLocals very late, just before profileOpts is run. Running deadTreesElimination after compactLocals might invalidate the analysis compactLocals has performed. The compactLocals analysis is only run if JitProfiling mode is not enabled.

This changes uses of profilingGroup to only take effect if JitProfilng mode is enabled, preventing deadTreesElimination from running after compactLocals. It also renames profilingGroup and profilingOpts to jitProfilingGroup and jitProfilingOpts to avoid confusion, as they really are specific to JitProfiling mode.

This change depends on OMR pull request eclipse-omr/omr#6782

Fixes #15569

[hzongaro]

Rahil @r30shah, I'm just running a personal build on this change to make sure there aren't any unexpected test failures, so I've marked it as a draft pull request. Once I mark it as ready-for-review, may I ask you to review this change?

[hzongaro]

I have revised this change to reflect the recommendations from @r30shah Rahil's review comments on OMR pull request #6782. I've made this as a draft pull request, as it depends on the revised OMR pull request eclipse-omr/omr#6782

[r30shah]

Changes looks good to me.

[hzongaro]

Jenkins test sanity all jdk8,jdk11,jdk17

[hzongaro]

Jenkins test sanity win32 jdk8

[hzongaro]

Vijay @vijaysun-omr, may I ask you to review this change? This fix takes advantage of the new IfJitProfiling option introduced by OMR pull request eclipse-omr/omr#6782

[vijaysun-omr]

While I don't have any issue with the logic in the commit as it stands, I did wonder about the "compact locals is only run if JIT profiling is not going to run" aspect. If this is just for saving some compile time, i.e. why bother running compact locals when the compile is only going to run for a relatively short time (as profiling compiles do) then I'd imagine a similar argument could apply to profiling compiles even when they use JProfiling. The only reason for bringing this up is that it might have changed the nature of the fix (where we may not have had to distinguish JIT profiling as such from JProfiling). If you prefer to deal with that in some other PR/issue, I may be okay with that, but I thought I would ask this question in any case.

[hzongaro]

I did wonder about the "compact locals is only run if JIT profiling is not going to run" aspect. If this is just for saving some compile time, i.e. why bother running compact locals when the compile is only going to run for a relatively short time (as profiling compiles do) then I'd imagine a similar argument could apply to profiling compiles even when they use JProfiling.

Viajy @vijaysun-omr, my understanding is that if JIT profiling is enabled, compact locals does not run for correctness reasons rather than to improve compile-time performance - the transformations performed by profilingGroup might invalidate the analysis performed by compact locals. Preventing compact locals from running if profiling was enabled was a limitation that was originally introduced shortly before OMR was contributed as an open source project. (I can provide a link to that commit off-line.)

When JProfiling was introduced, the condition under which compact locals runs was revised (in commit 25ae1d0) so that compact locals would not run under JIT profiling, but it would run under JProfiling or if neither profiling mode was enabled. profilingGroup, on the other hand, would be run under either of the profiling modes: both JIT profiling and JProfiling. Hence, if JProfiling was enabled, the transformations of profilingGroup (dead trees elimination, in particular) would be run after compact locals, potentially invalidating the analysis of compact locals, as was exposed in issue #15569.

[vijaysun-omr]

Thanks for the clarification (and the edit to the above comment to make it even easier to follow). Merging.