RFR (sh/8): Backport: JDK-8222766: Shenandoah: streamline post-LRB CAS barrier (x86)

Roman Kennke rkennke at redhat.com
Mon Feb 24 10:35:45 UTC 2020


>> Unfortunately, our new cas-obj assembly doesn't always exit with
>> condition flags intact. But we *do* have code to fill the result
>> register. So we should use that. The C1 adjustments pass the result
>> register to the CAS instruction and use that directly and skip emitting
>> the trailing cmove in IR.
> 
> Ha. So this was the testing culprit?
> 
>  797       __ cas_obj(addr, cmp.result(), val.result(), new_register(T_OBJECT),
> new_register(T_OBJECT), result);
>  798       return; <--- ouch

No. Or, well, kinda. The testing culprit was that my original attempt
was lacking the ShenandoahCASBarrier check:

+    if (UseShenandoahGC && ShenandoahCASBarrier) { <- here
+      LIR_Opr result = rlock_result(x);
+      __ cas_obj(addr, cmp.result(), val.result(),
new_register(T_OBJECT), new_register(T_OBJECT), result);
+      return;
     } else

And thus, when running with passive mode, it would generate the CAS IR
normally and return (omitting the trailing cmove). Then in LIRAssembler,
it would skip generating the Shenandoah CAS and emit the normal CAS --
without the trailing cmove. It's all quite a mess and I'm very happy we
have gotten rid of it in 11+.

> Suggestion:
> 
>   __ cas_obj(addr, cmp.result(), val.result(), new_register(T_OBJECT), new_register(T_OBJECT), result);
>   // Shenandoah C1 barrier would do all result management itself, shortcut here.
>   return;

I'll add the comment.

>> Testing: hotspot_gc_shenandoah passes
>> Might be useful if Aleksey or somebody else could run this on 32bit. My
>> own 32bit setup seems broken ATM.
> 
> My cross-compilation setup is also a bit broken, so two native tests cannot compile and run:
>  FAILED: gc/shenandoah/jni/TestCriticalNativeArgs.sh
>  FAILED: gc/shenandoah/jni/TestCriticalNativeStress.sh
> 
> This is not caused by the patch, and I don't believe those tests would be affected anyway.
> 
>> http://cr.openjdk.java.net/~rkennke/JDK-8222766-jdk8/webrev.00/
> 
> The shenandoahBarrierSetAssembler_x86.cpp change was transplated as is, right? I did not spot
> substantial differences against jdk/jdk, except minor changes around gc_state access and mark word
> constants.

Yes, other than those it's pretty much copy+paste from 11u.

Notice that in 8, we don't actually need the exchange path(s) - they
don't exist/are never used. I left them in for the sake of keeping the
code equivalent with 11.

Roman




More information about the shenandoah-dev mailing list