RFR: 8322743: C2: prevent elimination OSR locking region [v3]
Dean Long
dlong at openjdk.org
Wed Feb 28 22:50:48 UTC 2024
On Wed, 28 Feb 2024 21:47:08 GMT, Vladimir Kozlov <kvn at openjdk.org> wrote:
>> Corner case with a local (not escaped) object used for synchronization. C2 Escape Analysis thinks that it can eliminate locks for it. In most cases it is true but not in this case.
>>
>>
>> for (int i = 0; i < 2; ++i) {
>> Object o = new Object();
>> synchronized (o) { // monitorenter
>> // Trigger OSR compilation
>> for (int j = 0; j < 100_000; ++j) {
>>
>> The test has nested loop which trigger OSR compilation. The locked object comes from Interpreter into compiled OSR code. During parsing C2 creates an other non escaped object and correctly merge both together (with Phi node) so that non escaped object is not scalar replaceable. Because it does not globally escapes EA still removes locks for it and, as result, also for merged locked object from Interpreter which is the bug.
>>
>> The fix is to mark BoxLock node associated with OSR entry as Unbalanced to prevent EA from removing locks/unlocks from it. It is based on [JDK-8324969](https://github.com/openjdk/jdk/pull/17697) changes.
>>
>> Added regression test prepared by @TobiHartmann. Tested tier1-5, xcomp and stress.
>> Performance testing show no difference.
>
> Vladimir Kozlov has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains seven commits:
>
> - Update implementation
> - Merge branch 'master' into 8322743
> - Merge branch 'master' into 8322743
> - Backout original implementation
> - Address review comments
> - Fix trailing and other spaces.
> - 8322743: assert(held_monitor_count() == jni_monitor_count()) failed
I like this simpler version.
-------------
Marked as reviewed by dlong (Reviewer).
PR Review: https://git.openjdk.org/jdk/pull/17331#pullrequestreview-1907599840
More information about the hotspot-compiler-dev
mailing list