Details
-
Bug
-
Resolution: Not an Issue
-
P2
-
None
-
None
Description
Shenandoah will merge two gc state tests (from two LRBs) if they satisfy certain control flow constraint (in ShenandoahBarrierC2Support::merge_back_to_back_tests). I think there could be an issue if the two loads the two LRBs guard are dependent.
To reproduce, I ran java -XX:-UseCompressedOops -XX:+UseShenandoahGC -XX:CompileCommand=print,*ThreadLocal.set -jar SPECjvm2008.jar -coe -ict -ikv -i 2 -bt 8 derby
The C2 outcome for ThreadLocal.set has:
..
0: mov 0x268(%r15),%rsi ; Thread t = Thread.currentThread(); // get oop handle
1: mov (%rsi),%r12 ; Thread t = Thread.currentThread(); // get oop out of oop handle
2: mov 0x58(%r12),%rbx ; ThreadLocalMap map = getMap(t);
3: nopl 0x0(%rax)
4: testb $0x1,0x20(%r15) ; gc state test is too late here
5: jne 0x00007f0a3508f6e7
..
There is no barrier between line 1 and line 2. There was a barrier, but it was merged with the barrier for line 2, (the gc state tests are merged) and the merged test is placed at line 4. The problem is line 2 could visit from-space object and get outdated data.
If I comment out the call to ShenandoahBarrierC2Support::merge_back_to_back_tests, the outcome is more like what I expected:
..
0: mov 0x268(%r15),%rsi
1: mov (%rsi),%r12
2: nopw 0x0(%rax,%rax,1)
3: testb $0x1,0x20(%r15) ; first gc state test
4: jne 0x00007ff55d077207
5: mov 0x58(%r12),%rbx
6: testb $0x1,0x20(%r15) ; second gc state test
7: jne 0x00007ff55d077235
..
I think the merging didn't consider that the second load could use the result from the first load (in this case, r12). Can anyone confirm if this is an issue?
To reproduce, I ran java -XX:-UseCompressedOops -XX:+UseShenandoahGC -XX:CompileCommand=print,*ThreadLocal.set -jar SPECjvm2008.jar -coe -ict -ikv -i 2 -bt 8 derby
The C2 outcome for ThreadLocal.set has:
..
0: mov 0x268(%r15),%rsi ; Thread t = Thread.currentThread(); // get oop handle
1: mov (%rsi),%r12 ; Thread t = Thread.currentThread(); // get oop out of oop handle
2: mov 0x58(%r12),%rbx ; ThreadLocalMap map = getMap(t);
3: nopl 0x0(%rax)
4: testb $0x1,0x20(%r15) ; gc state test is too late here
5: jne 0x00007f0a3508f6e7
..
There is no barrier between line 1 and line 2. There was a barrier, but it was merged with the barrier for line 2, (the gc state tests are merged) and the merged test is placed at line 4. The problem is line 2 could visit from-space object and get outdated data.
If I comment out the call to ShenandoahBarrierC2Support::merge_back_to_back_tests, the outcome is more like what I expected:
..
0: mov 0x268(%r15),%rsi
1: mov (%rsi),%r12
2: nopw 0x0(%rax,%rax,1)
3: testb $0x1,0x20(%r15) ; first gc state test
4: jne 0x00007ff55d077207
5: mov 0x58(%r12),%rbx
6: testb $0x1,0x20(%r15) ; second gc state test
7: jne 0x00007ff55d077235
..
I think the merging didn't consider that the second load could use the result from the first load (in this case, r12). Can anyone confirm if this is an issue?