[aarch64-port-dev ] Correct memory barriers
Andrew Haley
aph at redhat.com
Mon May 19 11:22:06 UTC 2014
We weren't placing memory barriers correctly around locks; fixed
thusly.
Also, I've taken the opportunity to tidy up the memory barrier code
in the assembler.
Andrew.
# HG changeset patch
# User aph
# Date 1400498360 14400
# Mon May 19 07:19:20 2014 -0400
# Node ID a1363bc1be1dbe8a3bbe72732c30a27487753ed6
# Parent f1eaccaeed618cb852a652c18d343e7f1beffef6
Correct and tidy up memory barriers.
diff -r f1eaccaeed61 -r a1363bc1be1d src/cpu/aarch64/vm/aarch64.ad
--- a/src/cpu/aarch64/vm/aarch64.ad Fri May 16 11:42:50 2014 -0400
+++ b/src/cpu/aarch64/vm/aarch64.ad Mon May 19 07:19:20 2014 -0400
@@ -3143,11 +3143,10 @@
__ ldr(disp_hdr, Address(tmp, ObjectMonitor::cxq_offset_in_bytes()));
__ orr(rscratch1, rscratch1, disp_hdr); // Will be 0 if both are 0.
__ cmp(rscratch1, zr);
- __ br(Assembler::NE, cont);
+ __ cbnz(rscratch1, cont);
// need a release store here
__ lea(tmp, Address(tmp, ObjectMonitor::owner_offset_in_bytes()));
- __ mov(zr, rscratch1);
- __ stlr(rscratch1, tmp);
+ __ stlr(rscratch1, tmp); // rscratch1 is zero
}
__ bind(cont);
@@ -5818,14 +5817,14 @@
// ============================================================================
// MemBar Instruction
-instruct membar_acquire() %{
+instruct load_fence() %{
match(LoadFence);
ins_cost(VOLATILE_REF_COST);
format %{ "membar_acquire" %}
ins_encode %{
- __ membar(Assembler::Membar_mask_bits(Assembler::LoadLoad|Assembler::LoadStore));
+ __ membar(Assembler::LoadLoad|Assembler::LoadStore);
%}
ins_pipe(pipe_class_memory);
%}
@@ -5834,7 +5833,7 @@
match(MemBarAcquire);
ins_cost(0);
- format %{ " -- \t// redundant MEMBAR-acquire - empty" %}
+ format %{ "membar_acquire (elided)" %}
ins_encode %{
__ block_comment("membar_acquire (elided)");
@@ -5845,12 +5844,12 @@
instruct membar_acquire_lock() %{
match(MemBarAcquireLock);
- ins_cost(0);
-
- format %{ " -- \t// redundant MEMBAR-acquire - empty (acquire as part of CAS in prior FastLock)" %}
-
- ins_encode %{
- __ block_comment("membar_acquire_lock (elided)");
+ ins_cost(VOLATILE_REF_COST);
+
+ format %{ "membar_acquire_lock" %}
+
+ ins_encode %{
+ __ membar(Assembler::LoadLoad|Assembler::LoadStore);
%}
ins_pipe(pipe_class_memory);
@@ -5863,7 +5862,7 @@
format %{ "store_fence" %}
ins_encode %{
- __ membar(Assembler::AnyAny);
+ __ membar(Assembler::StoreLoad|Assembler::StoreStore);
%}
ins_pipe(pipe_class_memory);
%}
@@ -5872,7 +5871,7 @@
match(MemBarRelease);
ins_cost(0);
- format %{ "membar_release" %}
+ format %{ "membar_release (elided)" %}
ins_encode %{
__ block_comment("membar_release (elided)");
@@ -5894,10 +5893,12 @@
instruct membar_release_lock() %{
match(MemBarReleaseLock);
- ins_cost(0);
-
- ins_encode %{
- __ block_comment("membar_release_lock (elided)");
+ ins_cost(VOLATILE_REF_COST);
+
+ format %{ "membar_release_lock" %}
+
+ ins_encode %{
+ __ membar(Assembler::StoreLoad|Assembler::StoreStore);
%}
ins_pipe(pipe_class_memory);
@@ -5916,36 +5917,6 @@
ins_pipe(pipe_class_memory);
%}
-// This optimization is wrong on PPC and may be wrong on AArch64. The
-// following pattern is not supported:
-// MemBarVolatile
-// ^ ^
-// | |
-// CtrlProj MemProj
-// ^ ^
-// | |
-// | Load
-// |
-// MemBarVolatile
-//
-// The first MemBarVolatile could get optimized out! According to
-// Vladimir, this pattern can not occur on Oracle platforms.
-// However, it does occur on PPC64 (because of membars in
-// inline_unsafe_load_store).
-//
-// instruct unnecessary_membar_volatile() %{
-// match(MemBarVolatile);
-// predicate(Matcher::post_store_load_barrier(n));
-// ins_cost(0);
-
-// format %{ "!MEMBAR-volatile (unnecessary so empty encoding)" %}
-// ins_encode %{
-// __ block_comment("unnecessary_membar_volatile");
-// __ membar(Assembler::AnyAny);
-// %}
-// ins_pipe(pipe_class_empty);
-// %}
-
// ============================================================================
// Cast/Convert Instructions
diff -r f1eaccaeed61 -r a1363bc1be1d src/cpu/aarch64/vm/assembler_aarch64.hpp
--- a/src/cpu/aarch64/vm/assembler_aarch64.hpp Fri May 16 11:42:50 2014 -0400
+++ b/src/cpu/aarch64/vm/assembler_aarch64.hpp Mon May 19 07:19:20 2014 -0400
@@ -1992,6 +1992,11 @@
void emit_data64(jlong data, RelocationHolder const& rspec, int format = 0);
};
+inline Assembler::Membar_mask_bits operator|(Assembler::Membar_mask_bits a,
+ Assembler::Membar_mask_bits b) {
+ return Assembler::Membar_mask_bits(unsigned(a)|unsigned(b));
+}
+
Instruction_aarch64::~Instruction_aarch64() {
assem->emit();
}
@@ -2003,8 +2008,6 @@
return Assembler::Condition(int(cond) ^ 1);
}
-// extra stuff needed to compile
-// not sure which of these methods are really necessary
class BiasedLockingCounters;
extern "C" void das(uint64_t start, int len);
diff -r f1eaccaeed61 -r a1363bc1be1d src/cpu/aarch64/vm/c1_Runtime1_aarch64.cpp
--- a/src/cpu/aarch64/vm/c1_Runtime1_aarch64.cpp Fri May 16 11:42:50 2014 -0400
+++ b/src/cpu/aarch64/vm/c1_Runtime1_aarch64.cpp Mon May 19 07:19:20 2014 -0400
@@ -1257,7 +1257,7 @@
assert((int)CardTableModRefBS::dirty_card_val() == 0, "must be 0");
- __ membar(Assembler::Membar_mask_bits(Assembler::StoreLoad));
+ __ membar(Assembler::StoreLoad);
__ ldrb(rscratch1, Address(card_addr, offset));
__ cbzw(rscratch1, done);
diff -r f1eaccaeed61 -r a1363bc1be1d src/cpu/aarch64/vm/globalDefinitions_aarch64.hpp
--- a/src/cpu/aarch64/vm/globalDefinitions_aarch64.hpp Fri May 16 11:42:50 2014 -0400
+++ b/src/cpu/aarch64/vm/globalDefinitions_aarch64.hpp Mon May 19 07:19:20 2014 -0400
@@ -37,7 +37,4 @@
#define SUPPORTS_NATIVE_CX8
-// AArch64 is NOT multiple-copy-atomic.
-#define CPU_NOT_MULTIPLE_COPY_ATOMIC
-
#endif // CPU_AARCH64_VM_GLOBALDEFINITIONS_AARCH64_HPP
diff -r f1eaccaeed61 -r a1363bc1be1d src/cpu/aarch64/vm/macroAssembler_aarch64.cpp
--- a/src/cpu/aarch64/vm/macroAssembler_aarch64.cpp Fri May 16 11:42:50 2014 -0400
+++ b/src/cpu/aarch64/vm/macroAssembler_aarch64.cpp Mon May 19 07:19:20 2014 -0400
@@ -2520,7 +2520,7 @@
assert((int)CardTableModRefBS::dirty_card_val() == 0, "must be 0");
- membar(Assembler::Membar_mask_bits(Assembler::StoreLoad));
+ membar(Assembler::StoreLoad);
ldrb(tmp2, Address(card_addr, offset));
cbzw(tmp2, done);
diff -r f1eaccaeed61 -r a1363bc1be1d src/cpu/aarch64/vm/templateTable_aarch64.cpp
--- a/src/cpu/aarch64/vm/templateTable_aarch64.cpp Fri May 16 11:42:50 2014 -0400
+++ b/src/cpu/aarch64/vm/templateTable_aarch64.cpp Mon May 19 07:19:20 2014 -0400
@@ -2405,8 +2405,7 @@
__ bind(Done);
// It's really not worth bothering to check whether this field
// really is volatile in the slow case.
- __ membar(MacroAssembler::Membar_mask_bits(MacroAssembler::LoadLoad |
- MacroAssembler::LoadStore));
+ __ membar(MacroAssembler::LoadLoad | MacroAssembler::LoadStore);
}
@@ -2498,7 +2497,7 @@
{
Label notVolatile;
__ tbz(r5, ConstantPoolCacheEntry::is_volatile_shift, notVolatile);
- __ membar(MacroAssembler::Membar_mask_bits(MacroAssembler::StoreStore));
+ __ membar(MacroAssembler::StoreStore);
__ bind(notVolatile);
}
@@ -2645,7 +2644,7 @@
{
Label notVolatile;
__ tbz(r5, ConstantPoolCacheEntry::is_volatile_shift, notVolatile);
- __ membar(MacroAssembler::Membar_mask_bits(MacroAssembler::StoreLoad));
+ __ membar(MacroAssembler::StoreLoad);
__ bind(notVolatile);
}
}
@@ -2734,7 +2733,7 @@
{
Label notVolatile;
__ tbz(r3, ConstantPoolCacheEntry::is_volatile_shift, notVolatile);
- __ membar(MacroAssembler::Membar_mask_bits(MacroAssembler::StoreStore));
+ __ membar(MacroAssembler::StoreStore);
__ bind(notVolatile);
}
@@ -2778,7 +2777,7 @@
{
Label notVolatile;
__ tbz(r3, ConstantPoolCacheEntry::is_volatile_shift, notVolatile);
- __ membar(MacroAssembler::Membar_mask_bits(MacroAssembler::StoreLoad));
+ __ membar(MacroAssembler::StoreLoad);
__ bind(notVolatile);
}
}
@@ -2855,8 +2854,7 @@
{
Label notVolatile;
__ tbz(r3, ConstantPoolCacheEntry::is_volatile_shift, notVolatile);
- __ membar(MacroAssembler::Membar_mask_bits(MacroAssembler::LoadLoad |
- MacroAssembler::LoadStore));
+ __ membar(MacroAssembler::LoadLoad | MacroAssembler::LoadStore);
__ bind(notVolatile);
}
}
More information about the aarch64-port-dev
mailing list