Replace memory barriers to better reflect Java needs.

Replaces barriers that enforce ordering of one access type
(e.g. Load) with respect to another (e.g. store) with more general
ones that better reflect both Java requirements and actual hardware
barrier/fence instructions.  The old code was inconsistent and
unclear about which barriers implied which others.  Sometimes
multiple barriers were generated and then eliminated;
sometimes it was assumed that certain barriers implied others.
The new barriers closely parallel those in C++11, though, for now,
we use something closer to the old naming.

Bug: 14685856

Change-Id: Ie1c80afe3470057fc6f2b693a9831dfe83add831

15 files changed, 97 insertions, 85 deletions

diff --git a/compiler/dex/compiler_enums.h b/compiler/dex/compiler_enums.h
index 799a742032..bdedadbcf8 100644
--- a/compiler/dex/compiler_enums.h
+++ b/compiler/dex/compiler_enums.h
@@ -440,19 +440,23 @@ std::ostream& operator<<(std::ostream& os, const DividePattern& pattern);
 
 /**
  * @brief Memory barrier types (see "The JSR-133 Cookbook for Compiler Writers").
- * @details Without context sensitive analysis, the most conservative set of barriers
- * must be issued to ensure the Java Memory Model. Thus the recipe is as follows:
- * -# Use StoreStore barrier before volatile store.
- * -# Use StoreLoad barrier after volatile store.
- * -# Use LoadLoad and LoadStore barrier after each volatile load.
+ * @details We define the combined barrier types that are actually required
+ * by the Java Memory Model, rather than using exactly the terminology from
+ * the JSR-133 cookbook.  These should, in many cases, be replaced by acquire/release
+ * primitives.  Note that the JSR-133 cookbook generally does not deal with
+ * store atomicity issues, and the recipes there are not always entirely sufficient.
+ * The current recipe is as follows:
+ * -# Use AnyStore ~= (LoadStore | StoreStore) ~= release barrier before volatile store.
+ * -# Use AnyAny barrier after volatile store.  (StoreLoad is as expensive.)
+ * -# Use LoadAny barrier ~= (LoadLoad | LoadStore) ~= acquire barrierafter each volatile load.
  * -# Use StoreStore barrier after all stores but before return from any constructor whose
- * class has final fields.
+ *    class has final fields.
  */
 enum MemBarrierKind {
-  kLoadStore,
-  kLoadLoad,
+  kAnyStore,
+  kLoadAny,
   kStoreStore,
-  kStoreLoad
+  kAnyAny
 };
 
 std::ostream& operator<<(std::ostream& os, const MemBarrierKind& kind);
diff --git a/compiler/dex/quick/arm/call_arm.cc b/compiler/dex/quick/arm/call_arm.cc
index 04d6898e36..01e17bf44f 100644
--- a/compiler/dex/quick/arm/call_arm.cc
+++ b/compiler/dex/quick/arm/call_arm.cc
@@ -218,7 +218,7 @@ void ArmMir2Lir::GenMonitorEnter(int opt_flags, RegLocation rl_src) {
 
     LIR* success_target = NewLIR0(kPseudoTargetLabel);
     lock_success_branch->target = success_target;
-    GenMemBarrier(kLoadLoad);
+    GenMemBarrier(kLoadAny);
   } else {
     // Explicit null-check as slow-path is entered using an IT.
     GenNullCheck(rs_r0, opt_flags);
@@ -240,7 +240,7 @@ void ArmMir2Lir::GenMonitorEnter(int opt_flags, RegLocation rl_src) {
     LIR* call_inst = OpReg(kOpBlx/*ne*/, rs_rARM_LR);
     OpEndIT(it);
     MarkSafepointPC(call_inst);
-    GenMemBarrier(kLoadLoad);
+    GenMemBarrier(kLoadAny);
   }
 }
 
@@ -269,7 +269,7 @@ void ArmMir2Lir::GenMonitorExit(int opt_flags, RegLocation rl_src) {
     MarkPossibleNullPointerException(opt_flags);
     LoadConstantNoClobber(rs_r3, 0);
     LIR* slow_unlock_branch = OpCmpBranch(kCondNe, rs_r1, rs_r2, NULL);
-    GenMemBarrier(kStoreLoad);
+    GenMemBarrier(kAnyStore);
     Store32Disp(rs_r0, mirror::Object::MonitorOffset().Int32Value(), rs_r3);
     LIR* unlock_success_branch = OpUnconditionalBranch(NULL);
 
@@ -298,7 +298,7 @@ void ArmMir2Lir::GenMonitorExit(int opt_flags, RegLocation rl_src) {
     OpRegReg(kOpCmp, rs_r1, rs_r2);
 
     LIR* it = OpIT(kCondEq, "EE");
-    if (GenMemBarrier(kStoreLoad)) {
+    if (GenMemBarrier(kAnyStore)) {
       UpdateIT(it, "TEE");
     }
     Store32Disp/*eq*/(rs_r0, mirror::Object::MonitorOffset().Int32Value(), rs_r3);
diff --git a/compiler/dex/quick/arm/int_arm.cc b/compiler/dex/quick/arm/int_arm.cc
index 95071d92b2..f4ea592781 100644
--- a/compiler/dex/quick/arm/int_arm.cc
+++ b/compiler/dex/quick/arm/int_arm.cc
@@ -764,6 +764,7 @@ void ArmMir2Lir::OpTlsCmp(ThreadOffset<8> offset, int val) {
   UNIMPLEMENTED(FATAL) << "Should not be called.";
 }
 
+// Generate a CAS with memory_order_seq_cst semantics.
 bool ArmMir2Lir::GenInlinedCas(CallInfo* info, bool is_long, bool is_object) {
   DCHECK_EQ(cu_->instruction_set, kThumb2);
   // Unused - RegLocation rl_src_unsafe = info->args[0];
@@ -818,8 +819,8 @@ bool ArmMir2Lir::GenInlinedCas(CallInfo* info, bool is_long, bool is_object) {
     }
   }
 
-  // Release store semantics, get the barrier out of the way.  TODO: revisit
-  GenMemBarrier(kStoreLoad);
+  // Prevent reordering with prior memory operations.
+  GenMemBarrier(kAnyStore);
 
   RegLocation rl_object = LoadValue(rl_src_obj, kRefReg);
   RegLocation rl_new_value;
@@ -908,6 +909,9 @@ bool ArmMir2Lir::GenInlinedCas(CallInfo* info, bool is_long, bool is_object) {
     FreeTemp(rl_expected.reg);  // Now unneeded.
   }
 
+  // Prevent reordering with subsequent memory operations.
+  GenMemBarrier(kLoadAny);
+
   // result := (tmp1 != 0) ? 0 : 1;
   RegLocation rl_result = EvalLoc(rl_dest, kCoreReg, true);
   OpRegRegImm(kOpRsub, rl_result.reg, r_tmp, 1);
@@ -987,10 +991,10 @@ bool ArmMir2Lir::GenMemBarrier(MemBarrierKind barrier_kind) {
   int dmb_flavor;
   // TODO: revisit Arm barrier kinds
   switch (barrier_kind) {
-    case kLoadStore: dmb_flavor = kISH; break;
-    case kLoadLoad: dmb_flavor = kISH; break;
+    case kAnyStore: dmb_flavor = kISH; break;
+    case kLoadAny: dmb_flavor = kISH; break;
     case kStoreStore: dmb_flavor = kISHST; break;
-    case kStoreLoad: dmb_flavor = kISH; break;
+    case kAnyAny: dmb_flavor = kISH; break;
     default:
       LOG(FATAL) << "Unexpected MemBarrierKind: " << barrier_kind;
       dmb_flavor = kSY;  // quiet gcc.
diff --git a/compiler/dex/quick/arm/utility_arm.cc b/compiler/dex/quick/arm/utility_arm.cc
index 2d5e291442..9cbf7b89f2 100644
--- a/compiler/dex/quick/arm/utility_arm.cc
+++ b/compiler/dex/quick/arm/utility_arm.cc
@@ -986,10 +986,7 @@ LIR* ArmMir2Lir::LoadBaseDisp(RegStorage r_base, int displacement, RegStorage r_
   }
 
   if (UNLIKELY(is_volatile == kVolatile)) {
-    // Without context sensitive analysis, we must issue the most conservative barriers.
-    // In this case, either a load or store may follow so we issue both barriers.
-    GenMemBarrier(kLoadLoad);
-    GenMemBarrier(kLoadStore);
+    GenMemBarrier(kLoadAny);
   }
 
   return load;
@@ -1091,8 +1088,8 @@ LIR* ArmMir2Lir::StoreBaseDispBody(RegStorage r_base, int displacement, RegStora
 LIR* ArmMir2Lir::StoreBaseDisp(RegStorage r_base, int displacement, RegStorage r_src,
                                OpSize size, VolatileKind is_volatile) {
   if (UNLIKELY(is_volatile == kVolatile)) {
-    // There might have been a store before this volatile one so insert StoreStore barrier.
-    GenMemBarrier(kStoreStore);
+    // Ensure that prior accesses become visible to other threads first.
+    GenMemBarrier(kAnyStore);
   }
 
   LIR* store;
@@ -1135,8 +1132,9 @@ LIR* ArmMir2Lir::StoreBaseDisp(RegStorage r_base, int displacement, RegStorage r
   }
 
   if (UNLIKELY(is_volatile == kVolatile)) {
-    // A load might follow the volatile store so insert a StoreLoad barrier.
-    GenMemBarrier(kStoreLoad);
+    // Preserve order with respect to any subsequent volatile loads.
+    // We need StoreLoad, but that generally requires the most expensive barrier.
+    GenMemBarrier(kAnyAny);
   }
 
   return store;
diff --git a/compiler/dex/quick/arm64/arm64_lir.h b/compiler/dex/quick/arm64/arm64_lir.h
index 7490646b9b..ef4c0dedfb 100644
--- a/compiler/dex/quick/arm64/arm64_lir.h
+++ b/compiler/dex/quick/arm64/arm64_lir.h
@@ -376,6 +376,7 @@ enum ArmOpDmbOptions {
   kST = 0xe,
   kISH = 0xb,
   kISHST = 0xa,
+  kISHLD = 0x9,
   kNSH = 0x7,
   kNSHST = 0x6
 };
diff --git a/compiler/dex/quick/arm64/call_arm64.cc b/compiler/dex/quick/arm64/call_arm64.cc
index 56dcbe59e9..d24f419b09 100644
--- a/compiler/dex/quick/arm64/call_arm64.cc
+++ b/compiler/dex/quick/arm64/call_arm64.cc
@@ -228,7 +228,7 @@ void Arm64Mir2Lir::GenMonitorEnter(int opt_flags, RegLocation rl_src) {
 
   LIR* success_target = NewLIR0(kPseudoTargetLabel);
   lock_success_branch->target = success_target;
-  GenMemBarrier(kLoadLoad);
+  GenMemBarrier(kLoadAny);
 }
 
 /*
@@ -258,7 +258,7 @@ void Arm64Mir2Lir::GenMonitorExit(int opt_flags, RegLocation rl_src) {
   Load32Disp(rs_x0, mirror::Object::MonitorOffset().Int32Value(), rs_w2);
   MarkPossibleNullPointerException(opt_flags);
   LIR* slow_unlock_branch = OpCmpBranch(kCondNe, rs_w1, rs_w2, NULL);
-  GenMemBarrier(kStoreLoad);
+  GenMemBarrier(kAnyStore);
   Store32Disp(rs_x0, mirror::Object::MonitorOffset().Int32Value(), rs_wzr);
   LIR* unlock_success_branch = OpUnconditionalBranch(NULL);
 
diff --git a/compiler/dex/quick/arm64/int_arm64.cc b/compiler/dex/quick/arm64/int_arm64.cc
index a7ca685fd5..f7aa39f4c2 100644
--- a/compiler/dex/quick/arm64/int_arm64.cc
+++ b/compiler/dex/quick/arm64/int_arm64.cc
@@ -740,10 +740,16 @@ bool Arm64Mir2Lir::GenMemBarrier(MemBarrierKind barrier_kind) {
   int dmb_flavor;
   // TODO: revisit Arm barrier kinds
   switch (barrier_kind) {
-    case kLoadStore: dmb_flavor = kISH; break;
-    case kLoadLoad: dmb_flavor = kISH; break;
+    case kAnyStore: dmb_flavor = kISH; break;
+    case kLoadAny: dmb_flavor = kISH; break;
+        // We conjecture that kISHLD is insufficient.  It is documented
+        // to provide LoadLoad | StoreStore ordering.  But if this were used
+        // to implement volatile loads, we suspect that the lack of store
+        // atomicity on ARM would cause us to allow incorrect results for
+        // the canonical IRIW example.  But we're not sure.
+        // We should be using acquire loads instead.
     case kStoreStore: dmb_flavor = kISHST; break;
-    case kStoreLoad: dmb_flavor = kISH; break;
+    case kAnyAny: dmb_flavor = kISH; break;
     default:
       LOG(FATAL) << "Unexpected MemBarrierKind: " << barrier_kind;
       dmb_flavor = kSY;  // quiet gcc.
diff --git a/compiler/dex/quick/arm64/utility_arm64.cc b/compiler/dex/quick/arm64/utility_arm64.cc
index 22a4ec4d49..097fcdca03 100644
--- a/compiler/dex/quick/arm64/utility_arm64.cc
+++ b/compiler/dex/quick/arm64/utility_arm64.cc
@@ -1145,10 +1145,8 @@ LIR* Arm64Mir2Lir::LoadBaseDisp(RegStorage r_base, int displacement, RegStorage
   LIR* load = LoadBaseDispBody(r_base, displacement, r_dest, size);
 
   if (UNLIKELY(is_volatile == kVolatile)) {
-    // Without context sensitive analysis, we must issue the most conservative barriers.
-    // In this case, either a load or store may follow so we issue both barriers.
-    GenMemBarrier(kLoadLoad);
-    GenMemBarrier(kLoadStore);
+    // TODO: This should generate an acquire load instead of the barrier.
+    GenMemBarrier(kLoadAny);
   }
 
   return load;
@@ -1232,9 +1230,10 @@ LIR* Arm64Mir2Lir::StoreBaseDispBody(RegStorage r_base, int displacement, RegSto
 
 LIR* Arm64Mir2Lir::StoreBaseDisp(RegStorage r_base, int displacement, RegStorage r_src,
                                  OpSize size, VolatileKind is_volatile) {
+  // TODO: This should generate a release store and no barriers.
   if (UNLIKELY(is_volatile == kVolatile)) {
-    // There might have been a store before this volatile one so insert StoreStore barrier.
-    GenMemBarrier(kStoreStore);
+    // Ensure that prior accesses become visible to other threads first.
+    GenMemBarrier(kAnyStore);
   }
 
   // StoreBaseDisp() will emit correct insn for atomic store on arm64
@@ -1243,8 +1242,9 @@ LIR* Arm64Mir2Lir::StoreBaseDisp(RegStorage r_base, int displacement, RegStorage
   LIR* store = StoreBaseDispBody(r_base, displacement, r_src, size);
 
   if (UNLIKELY(is_volatile == kVolatile)) {
-    // A load might follow the volatile store so insert a StoreLoad barrier.
-    GenMemBarrier(kStoreLoad);
+    // Preserve order with respect to any subsequent volatile loads.
+    // We need StoreLoad, but that generally requires the most expensive barrier.
+    GenMemBarrier(kAnyAny);
   }
 
   return store;
diff --git a/compiler/dex/quick/gen_common.cc b/compiler/dex/quick/gen_common.cc
index 6dc019ac53..c266a3c2e9 100644
--- a/compiler/dex/quick/gen_common.cc
+++ b/compiler/dex/quick/gen_common.cc
@@ -629,8 +629,12 @@ void Mir2Lir::GenSput(MIR* mir, RegLocation rl_src, bool is_long_or_double,
                                                      field_info.StorageIndex(), r_base));
 
         FreeTemp(r_tmp);
-        // Ensure load of status and load of value don't re-order.
-        GenMemBarrier(kLoadLoad);
+        // Ensure load of status and store of value don't re-order.
+        // TODO: Presumably the actual value store is control-dependent on the status load,
+        // and will thus not be reordered in any case, since stores are never speculated.
+        // Does later code "know" that the class is now initialized?  If so, we still
+        // need the barrier to guard later static loads.
+        GenMemBarrier(kLoadAny);
       }
       FreeTemp(r_method);
     }
@@ -723,7 +727,7 @@ void Mir2Lir::GenSget(MIR* mir, RegLocation rl_dest,
 
         FreeTemp(r_tmp);
         // Ensure load of status and load of value don't re-order.
-        GenMemBarrier(kLoadLoad);
+        GenMemBarrier(kLoadAny);
       }
       FreeTemp(r_method);
     }
diff --git a/compiler/dex/quick/gen_invoke.cc b/compiler/dex/quick/gen_invoke.cc
index 6c0dfe80a6..56986b4b6f 100755
--- a/compiler/dex/quick/gen_invoke.cc
+++ b/compiler/dex/quick/gen_invoke.cc
@@ -1711,10 +1711,7 @@ bool Mir2Lir::GenInlinedUnsafeGet(CallInfo* info,
   }
 
   if (is_volatile) {
-    // Without context sensitive analysis, we must issue the most conservative barriers.
-    // In this case, either a load or store may follow so we issue both barriers.
-    GenMemBarrier(kLoadLoad);
-    GenMemBarrier(kLoadStore);
+    GenMemBarrier(kLoadAny);
   }
 
   if (is_long) {
@@ -1737,8 +1734,7 @@ bool Mir2Lir::GenInlinedUnsafePut(CallInfo* info, bool is_long,
   rl_src_offset = NarrowRegLoc(rl_src_offset);  // ignore high half in info->args[3]
   RegLocation rl_src_value = info->args[4];  // value to store
   if (is_volatile || is_ordered) {
-    // There might have been a store before this volatile one so insert StoreStore barrier.
-    GenMemBarrier(kStoreStore);
+    GenMemBarrier(kAnyStore);
   }
   RegLocation rl_object = LoadValue(rl_src_obj, kRefReg);
   RegLocation rl_offset = LoadValue(rl_src_offset, kCoreReg);
@@ -1767,8 +1763,9 @@ bool Mir2Lir::GenInlinedUnsafePut(CallInfo* info, bool is_long,
   FreeTemp(rl_offset.reg);
 
   if (is_volatile) {
-    // A load might follow the volatile store so insert a StoreLoad barrier.
-    GenMemBarrier(kStoreLoad);
+    // Prevent reordering with a subsequent volatile load.
+    // May also be needed to address store atomicity issues.
+    GenMemBarrier(kAnyAny);
   }
   if (is_object) {
     MarkGCCard(rl_value.reg, rl_object.reg);
diff --git a/compiler/dex/quick/mips/utility_mips.cc b/compiler/dex/quick/mips/utility_mips.cc
index 129a696625..75d3c5d4bf 100644
--- a/compiler/dex/quick/mips/utility_mips.cc
+++ b/compiler/dex/quick/mips/utility_mips.cc
@@ -563,10 +563,7 @@ LIR* MipsMir2Lir::LoadBaseDisp(RegStorage r_base, int displacement, RegStorage r
   load = LoadBaseDispBody(r_base, displacement, r_dest, size);
 
   if (UNLIKELY(is_volatile == kVolatile)) {
-    // Without context sensitive analysis, we must issue the most conservative barriers.
-    // In this case, either a load or store may follow so we issue both barriers.
-    GenMemBarrier(kLoadLoad);
-    GenMemBarrier(kLoadStore);
+    GenMemBarrier(kLoadAny);
   }
 
   return load;
@@ -658,8 +655,8 @@ LIR* MipsMir2Lir::StoreBaseDisp(RegStorage r_base, int displacement, RegStorage
                                 OpSize size, VolatileKind is_volatile) {
   if (is_volatile == kVolatile) {
     DCHECK(size != k64 && size != kDouble);
-    // There might have been a store before this volatile one so insert StoreStore barrier.
-    GenMemBarrier(kStoreStore);
+    // Ensure that prior accesses become visible to other threads first.
+    GenMemBarrier(kAnyStore);
   }
 
   // TODO: base this on target.
@@ -670,8 +667,9 @@ LIR* MipsMir2Lir::StoreBaseDisp(RegStorage r_base, int displacement, RegStorage
   store = StoreBaseDispBody(r_base, displacement, r_src, size);
 
   if (UNLIKELY(is_volatile == kVolatile)) {
-    // A load might follow the volatile store so insert a StoreLoad barrier.
-    GenMemBarrier(kStoreLoad);
+    // Preserve order with respect to any subsequent volatile loads.
+    // We need StoreLoad, but that generally requires the most expensive barrier.
+    GenMemBarrier(kAnyAny);
   }
 
   return store;
diff --git a/compiler/dex/quick/x86/int_x86.cc b/compiler/dex/quick/x86/int_x86.cc
index f1166f6263..4ecc5d8673 100755
--- a/compiler/dex/quick/x86/int_x86.cc
+++ b/compiler/dex/quick/x86/int_x86.cc
@@ -861,7 +861,7 @@ bool X86Mir2Lir::GenInlinedCas(CallInfo* info, bool is_long, bool is_object) {
 
     // After a store we need to insert barrier in case of potential load. Since the
     // locked cmpxchg has full barrier semantics, only a scheduling barrier will be generated.
-    GenMemBarrier(kStoreLoad);
+    GenMemBarrier(kAnyAny);
 
     FreeTemp(rs_r0q);
   } else if (is_long) {
@@ -913,10 +913,11 @@ bool X86Mir2Lir::GenInlinedCas(CallInfo* info, bool is_long, bool is_object) {
     }
     NewLIR4(kX86LockCmpxchg64A, rs_obj.GetReg(), rs_off.GetReg(), 0, 0);
 
-    // After a store we need to insert barrier in case of potential load. Since the
-    // locked cmpxchg has full barrier semantics, only a scheduling barrier will be generated.
-    GenMemBarrier(kStoreLoad);
-
+    // After a store we need to insert barrier to prevent reordering with either
+    // earlier or later memory accesses.  Since
+    // locked cmpxchg has full barrier semantics, only a scheduling barrier will be generated,
+    // and it will be associated with the cmpxchg instruction, preventing both.
+    GenMemBarrier(kAnyAny);
 
     if (push_si) {
       FreeTemp(rs_rSI);
@@ -954,9 +955,11 @@ bool X86Mir2Lir::GenInlinedCas(CallInfo* info, bool is_long, bool is_object) {
     LoadValueDirect(rl_src_expected, rs_r0);
     NewLIR5(kX86LockCmpxchgAR, rl_object.reg.GetReg(), rl_offset.reg.GetReg(), 0, 0, rl_new_value.reg.GetReg());
 
-    // After a store we need to insert barrier in case of potential load. Since the
-    // locked cmpxchg has full barrier semantics, only a scheduling barrier will be generated.
-    GenMemBarrier(kStoreLoad);
+    // After a store we need to insert barrier to prevent reordering with either
+    // earlier or later memory accesses.  Since
+    // locked cmpxchg has full barrier semantics, only a scheduling barrier will be generated,
+    // and it will be associated with the cmpxchg instruction, preventing both.
+    GenMemBarrier(kAnyAny);
 
     FreeTemp(rs_r0);
   }
diff --git a/compiler/dex/quick/x86/target_x86.cc b/compiler/dex/quick/x86/target_x86.cc
index 1ebbbbd5ee..4310525a5d 100755
--- a/compiler/dex/quick/x86/target_x86.cc
+++ b/compiler/dex/quick/x86/target_x86.cc
@@ -582,11 +582,11 @@ bool X86Mir2Lir::GenMemBarrier(MemBarrierKind barrier_kind) {
 
   bool ret = false;
   /*
-   * According to the JSR-133 Cookbook, for x86 only StoreLoad barriers need memory fence. All other barriers
-   * (LoadLoad, LoadStore, StoreStore) are nops due to the x86 memory model. For those cases, all we need
-   * to ensure is that there is a scheduling barrier in place.
+   * According to the JSR-133 Cookbook, for x86 only StoreLoad/AnyAny barriers need memory fence.
+   * All other barriers (LoadAny, AnyStore, StoreStore) are nops due to the x86 memory model.
+   * For those cases, all we need to ensure is that there is a scheduling barrier in place.
    */
-  if (barrier_kind == kStoreLoad) {
+  if (barrier_kind == kAnyAny) {
     // If no LIR exists already that can be used a barrier, then generate an mfence.
     if (mem_barrier == nullptr) {
       mem_barrier = NewLIR0(kX86Mfence);
diff --git a/compiler/dex/quick/x86/utility_x86.cc b/compiler/dex/quick/x86/utility_x86.cc
index 5c7c91b5b5..045e58e5a7 100644
--- a/compiler/dex/quick/x86/utility_x86.cc
+++ b/compiler/dex/quick/x86/utility_x86.cc
@@ -762,10 +762,7 @@ LIR* X86Mir2Lir::LoadBaseDisp(RegStorage r_base, int displacement, RegStorage r_
                                   size);
 
   if (UNLIKELY(is_volatile == kVolatile)) {
-    // Without context sensitive analysis, we must issue the most conservative barriers.
-    // In this case, either a load or store may follow so we issue both barriers.
-    GenMemBarrier(kLoadLoad);
-    GenMemBarrier(kLoadStore);
+    GenMemBarrier(kLoadAny);  // Only a scheduling barrier.
   }
 
   return load;
@@ -863,8 +860,7 @@ LIR* X86Mir2Lir::StoreBaseIndexed(RegStorage r_base, RegStorage r_index, RegStor
 LIR* X86Mir2Lir::StoreBaseDisp(RegStorage r_base, int displacement, RegStorage r_src, OpSize size,
                                VolatileKind is_volatile) {
   if (UNLIKELY(is_volatile == kVolatile)) {
-    // There might have been a store before this volatile one so insert StoreStore barrier.
-    GenMemBarrier(kStoreStore);
+    GenMemBarrier(kAnyStore);  // Only a scheduling barrier.
   }
 
   // StoreBaseDisp() will emit correct insn for atomic store on x86
@@ -873,8 +869,9 @@ LIR* X86Mir2Lir::StoreBaseDisp(RegStorage r_base, int displacement, RegStorage r
   LIR* store = StoreBaseIndexedDisp(r_base, RegStorage::InvalidReg(), 0, displacement, r_src, size);
 
   if (UNLIKELY(is_volatile == kVolatile)) {
-    // A load might follow the volatile store so insert a StoreLoad barrier.
-    GenMemBarrier(kStoreLoad);
+    // A volatile load might follow the volatile store so insert a StoreLoad barrier.
+    // This does require a fence, even on x86.
+    GenMemBarrier(kAnyAny);
   }
 
   return store;
diff --git a/compiler/llvm/gbc_expander.cc b/compiler/llvm/gbc_expander.cc
index f8dca66de0..902f8dd4f5 100644
--- a/compiler/llvm/gbc_expander.cc
+++ b/compiler/llvm/gbc_expander.cc
@@ -1648,7 +1648,7 @@ llvm::Value* GBCExpanderPass::Expand_HLIGet(llvm::CallInst& call_inst,
     field_value = SignOrZeroExtendCat1Types(field_value, field_jty);
 
     if (is_volatile) {
-      irb_.CreateMemoryBarrier(art::kLoadLoad);
+      irb_.CreateMemoryBarrier(art::kLoadAny);
     }
   }
 
@@ -1702,7 +1702,7 @@ void GBCExpanderPass::Expand_HLIPut(llvm::CallInst& call_inst,
     DCHECK_GE(field_offset.Int32Value(), 0);
 
     if (is_volatile) {
-      irb_.CreateMemoryBarrier(art::kStoreStore);
+      irb_.CreateMemoryBarrier(art::kAnyStore);
     }
 
     llvm::PointerType* field_type =
@@ -1717,7 +1717,7 @@ void GBCExpanderPass::Expand_HLIPut(llvm::CallInst& call_inst,
     irb_.CreateStore(new_value, field_addr, kTBAAHeapInstance, field_jty);
 
     if (is_volatile) {
-      irb_.CreateMemoryBarrier(art::kLoadLoad);
+      irb_.CreateMemoryBarrier(art::kAnyAny);
     }
 
     if (field_jty == kObject) {  // If put an object, mark the GC card table.
@@ -1870,7 +1870,7 @@ llvm::Value* GBCExpanderPass::EmitLoadStaticStorage(uint32_t dex_pc,
   phi->addIncoming(loaded_storage_object_addr, block_after_load_static);
 
   // Ensure load of status and load of value don't re-order.
-  irb_.CreateMemoryBarrier(art::kLoadLoad);
+  irb_.CreateMemoryBarrier(art::kLoadAny);
 
   return phi;
 }
@@ -1948,7 +1948,7 @@ llvm::Value* GBCExpanderPass::Expand_HLSget(llvm::CallInst& call_inst,
     static_field_value = SignOrZeroExtendCat1Types(static_field_value, field_jty);
 
     if (is_volatile) {
-      irb_.CreateMemoryBarrier(art::kLoadLoad);
+      irb_.CreateMemoryBarrier(art::kLoadAny);
     }
   }
 
@@ -2025,7 +2025,7 @@ void GBCExpanderPass::Expand_HLSput(llvm::CallInst& call_inst,
     }
 
     if (is_volatile) {
-      irb_.CreateMemoryBarrier(art::kStoreStore);
+      irb_.CreateMemoryBarrier(art::kAnyStore);
     }
 
     llvm::Value* static_field_offset_value = irb_.getPtrEquivInt(field_offset.Int32Value());
@@ -2038,7 +2038,7 @@ void GBCExpanderPass::Expand_HLSput(llvm::CallInst& call_inst,
     irb_.CreateStore(new_value, static_field_addr, kTBAAHeapStatic, field_jty);
 
     if (is_volatile) {
-      irb_.CreateMemoryBarrier(art::kStoreLoad);
+      irb_.CreateMemoryBarrier(art::kAnyAny);
     }
 
     if (field_jty == kObject) {  // If put an object, mark the GC card table.