8340796: Use a consistent order when loading cxq and EntryList

Reviewed-by: dholmes, coleenp, eosterlund

src/hotspot/cpu/aarch64/c2_MacroAssembler_aarch64.cpp

@@ -217,7 +217,7 @@ void C2_MacroAssembler::fast_unlock(Register objectReg, Register boxReg, Registe
   // StoreLoad achieves this.
   membar(StoreLoad);
 
-  // Check if the entry lists are empty.
+  // Check if the entry lists are empty (EntryList first - by convention).
   ldr(rscratch1, Address(tmp, ObjectMonitor::EntryList_offset()));
   ldr(tmpReg, Address(tmp, ObjectMonitor::cxq_offset()));
   orr(rscratch1, rscratch1, tmpReg);
@@ -538,7 +538,7 @@ void C2_MacroAssembler::fast_unlock_lightweight(Register obj, Register box, Regi
     // StoreLoad achieves this.
     membar(StoreLoad);
 
-    // Check if the entry lists are empty.
+    // Check if the entry lists are empty (EntryList first - by convention).
     ldr(rscratch1, Address(t1_monitor, ObjectMonitor::EntryList_offset()));
     ldr(t3_t, Address(t1_monitor, ObjectMonitor::cxq_offset()));
     orr(rscratch1, rscratch1, t3_t);

src/hotspot/cpu/ppc/macroAssembler_ppc.cpp

@@ -2736,7 +2736,7 @@ void MacroAssembler::compiler_fast_unlock_object(ConditionRegister flag, Registe
   // StoreLoad achieves this.
   membar(StoreLoad);
 
-  // Check if the entry lists are empty.
+  // Check if the entry lists are empty (EntryList first - by convention).
   ld(temp,             in_bytes(ObjectMonitor::EntryList_offset()), current_header);
   ld(displaced_header, in_bytes(ObjectMonitor::cxq_offset()), current_header);
   orr(temp, temp, displaced_header); // Will be 0 if both are 0.
@@ -3083,7 +3083,7 @@ void MacroAssembler::compiler_fast_unlock_lightweight_object(ConditionRegister f
     // StoreLoad achieves this.
     membar(StoreLoad);
 
-    // Check if the entry lists are empty.
+    // Check if the entry lists are empty (EntryList first - by convention).
     ld(t, in_bytes(ObjectMonitor::EntryList_offset()), monitor);
     ld(t2, in_bytes(ObjectMonitor::cxq_offset()), monitor);
     orr(t, t, t2);

src/hotspot/cpu/riscv/c2_MacroAssembler_riscv.cpp

@@ -234,7 +234,7 @@ void C2_MacroAssembler::fast_unlock(Register objectReg, Register boxReg,
   // StoreLoad achieves this.
   membar(StoreLoad);
 
-  // Check if the entry lists are empty.
+  // Check if the entry lists are empty (EntryList first - by convention).
   ld(t0, Address(tmp, ObjectMonitor::EntryList_offset()));
   ld(tmp1Reg, Address(tmp, ObjectMonitor::cxq_offset()));
   orr(t0, t0, tmp1Reg);
@@ -566,7 +566,7 @@ void C2_MacroAssembler::fast_unlock_lightweight(Register obj, Register box,
     // StoreLoad achieves this.
     membar(StoreLoad);
 
-    // Check if the entry lists are empty.
+    // Check if the entry lists are empty (EntryList first - by convention).
     ld(t0, Address(tmp1_monitor, ObjectMonitor::EntryList_offset()));
     ld(tmp3_t, Address(tmp1_monitor, ObjectMonitor::cxq_offset()));
     orr(t0, t0, tmp3_t);

src/hotspot/cpu/s390/macroAssembler_s390.cpp

@@ -3667,7 +3667,7 @@ void MacroAssembler::compiler_fast_unlock_object(Register oop, Register box, Reg
   // We need a full fence after clearing owner to avoid stranding.
   z_fence();
 
-  // Check if the entry lists are empty.
+  // Check if the entry lists are empty (EntryList first - by convention).
   load_and_test_long(temp, Address(currentHeader, OM_OFFSET_NO_MONITOR_VALUE_TAG(EntryList)));
   z_brne(check_succ);
   load_and_test_long(temp, Address(currentHeader, OM_OFFSET_NO_MONITOR_VALUE_TAG(cxq)));
@@ -6510,7 +6510,7 @@ void MacroAssembler::compiler_fast_unlock_lightweight_object(Register obj, Regis
     // We need a full fence after clearing owner to avoid stranding.
     z_fence();
 
-    // Check if the entry lists are empty.
+    // Check if the entry lists are empty (EntryList first - by convention).
     load_and_test_long(tmp2, EntryList_address);
     z_brne(check_succ);
     load_and_test_long(tmp2, cxq_address);

src/hotspot/cpu/x86/c2_MacroAssembler_x86.cpp

@@ -477,9 +477,9 @@ void C2_MacroAssembler::fast_unlock(Register objReg, Register boxReg, Register t
   // StoreLoad achieves this.
   membar(StoreLoad);
 
-  // Check if the entry lists are empty.
-  movptr(boxReg, Address(tmpReg, OM_OFFSET_NO_MONITOR_VALUE_TAG(cxq)));
-  orptr(boxReg, Address(tmpReg, OM_OFFSET_NO_MONITOR_VALUE_TAG(EntryList)));
+  // Check if the entry lists are empty (EntryList first - by convention).
+  movptr(boxReg, Address(tmpReg, OM_OFFSET_NO_MONITOR_VALUE_TAG(EntryList)));
+  orptr(boxReg, Address(tmpReg, OM_OFFSET_NO_MONITOR_VALUE_TAG(cxq)));
   jccb(Assembler::zero, LSuccess);    // If so we are done.
 
   // Check if there is a successor.
@@ -806,9 +806,9 @@ void C2_MacroAssembler::fast_unlock_lightweight(Register obj, Register reg_rax,
     // StoreLoad achieves this.
     membar(StoreLoad);
 
-    // Check if the entry lists are empty.
-    movptr(reg_rax, cxq_address);
-    orptr(reg_rax, EntryList_address);
+    // Check if the entry lists are empty (EntryList first - by convention).
+    movptr(reg_rax, EntryList_address);
+    orptr(reg_rax, cxq_address);
     jccb(Assembler::zero, unlocked);    // If so we are done.
 
     // Check if there is a successor.

src/hotspot/share/runtime/objectMonitor.cpp

@@ -1133,6 +1133,14 @@ void ObjectMonitor::UnlinkAfterAcquire(JavaThread* current, ObjectWaiter* curren
 // or drain _cxq, we need to reacquire the lock before we can wake up
 // (unpark) a waiting thread.
 //
+// Note that we read the EntryList and then the cxq after dropping the
+// lock, so the values need not form a stable snapshot. In particular,
+// after reading the (empty) EntryList, another thread could acquire
+// and release the lock, moving any entries in the cxq to the
+// EntryList, causing the current thread to see an empty cxq and
+// conclude there are no waiters. But this is okay as the thread that
+// moved the cxq is responsible for waking the successor.
+//
 // The CAS() in enter provides for safety and exclusion, while the
 // MEMBAR in exit provides for progress and avoids stranding.
 //