RFR: 8296136: Use correct register in aarch64_enc_fast_unlock()

Fei Yang fyang at openjdk.org
Tue Nov 1 06:56:27 UTC 2022


On Mon, 31 Oct 2022 17:31:31 GMT, Roman Kennke <rkennke at openjdk.org> wrote:

> In aarch64_enc_fast_unlock() (aarch64.ad) we have this piece of code:
> 
> 
>     __ ldr(tmp, Address(oop, oopDesc::mark_offset_in_bytes()));
>     __ tbnz(disp_hdr, exact_log2(markWord::monitor_value), object_has_monitor);
> 
> 
> The tbnz uses the wrong register - it should really use tmp. disp_hdr has been loaded with the displaced header of the stack-lock, which would never have its monitor bits set, thus the branch will always take the slow path. In this common case, it is only a performance nuisance. In the case of !UseHeavyMonitors it is even worse, then disp_hdr will be unitialized, and we are facing a correctness problem.
> 
> As far as I can tell, the problem dates back to when aarch64 C2 parts have been added to OpenJDK.
> 
> Testing:
>  - [x] tier1
>  - [x] tier2
>  - [x] tier3
>  - [ ] tier4

Could you please also incorporate following fix for RISC-V at the same time? I see it inherits the same similar issue here. 
This has passed tier1 test on HiFive Unmatched board. Thanks.


diff --git a/src/hotspot/cpu/riscv/riscv.ad b/src/hotspot/cpu/riscv/riscv.ad
index 75612ef7508..abe0f609a62 100644
--- a/src/hotspot/cpu/riscv/riscv.ad
+++ b/src/hotspot/cpu/riscv/riscv.ad
@@ -2474,7 +2474,7 @@ encode %{

     // Handle existing monitor.
     __ ld(tmp, Address(oop, oopDesc::mark_offset_in_bytes()));
-    __ andi(t0, disp_hdr, markWord::monitor_value);
+    __ andi(t0, tmp, markWord::monitor_value);
     __ bnez(t0, object_has_monitor);

     if (!UseHeavyMonitors) {

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

PR: https://git.openjdk.org/jdk/pull/10921


More information about the hotspot-compiler-dev mailing list