Quick: Fix LVN/GVN handling of acquire operations.
Acquire operations, i.e. MONITOR_ENTER and volatile GETs,
change the thread's view of the memory, so subsequent loads
must get new value names in LVN/GVN. Release operations do
not affect this thread's view of the memory, they the only
push the modifications for other threads to see.
Bug: 17689750
Change-Id: I9442d89b1d2c5252b99b02851b71bb85f871d734
diff --git a/compiler/dex/local_value_numbering.cc b/compiler/dex/local_value_numbering.cc
index 4279955..eb0806b 100644
--- a/compiler/dex/local_value_numbering.cc
+++ b/compiler/dex/local_value_numbering.cc
@@ -1169,8 +1169,9 @@
const MirFieldInfo& field_info = gvn_->GetMirGraph()->GetIFieldLoweringInfo(mir);
uint16_t res;
if (!field_info.IsResolved() || field_info.IsVolatile()) {
- // Volatile fields always get a new memory version; field id is irrelevant.
// Unresolved fields may be volatile, so handle them as such to be safe.
+ HandleInvokeOrClInitOrAcquireOp(mir); // Volatile GETs have acquire semantics.
+ // Volatile fields always get a new memory version; field id is irrelevant.
// Use result s_reg - will be unique.
res = gvn_->LookupValue(kNoValue, mir->ssa_rep->defs[0], kNoValue, kNoValue);
} else {
@@ -1269,14 +1270,16 @@
uint16_t LocalValueNumbering::HandleSGet(MIR* mir, uint16_t opcode) {
const MirSFieldLoweringInfo& field_info = gvn_->GetMirGraph()->GetSFieldLoweringInfo(mir);
- if (!field_info.IsInitialized() && (mir->optimization_flags & MIR_IGNORE_CLINIT_CHECK) == 0) {
- // Class initialization can call arbitrary functions, we need to wipe aliasing values.
- HandleInvokeOrClInit(mir);
+ if (!field_info.IsResolved() || field_info.IsVolatile() ||
+ (!field_info.IsInitialized() && (mir->optimization_flags & MIR_IGNORE_CLINIT_CHECK) == 0)) {
+ // Volatile SGETs (and unresolved fields are potentially volatile) have acquire semantics
+ // and class initialization can call arbitrary functions, we need to wipe aliasing values.
+ HandleInvokeOrClInitOrAcquireOp(mir);
}
uint16_t res;
if (!field_info.IsResolved() || field_info.IsVolatile()) {
- // Volatile fields always get a new memory version; field id is irrelevant.
// Unresolved fields may be volatile, so handle them as such to be safe.
+ // Volatile fields always get a new memory version; field id is irrelevant.
// Use result s_reg - will be unique.
res = gvn_->LookupValue(kNoValue, mir->ssa_rep->defs[0], kNoValue, kNoValue);
} else {
@@ -1306,7 +1309,7 @@
const MirSFieldLoweringInfo& field_info = gvn_->GetMirGraph()->GetSFieldLoweringInfo(mir);
if (!field_info.IsInitialized() && (mir->optimization_flags & MIR_IGNORE_CLINIT_CHECK) == 0) {
// Class initialization can call arbitrary functions, we need to wipe aliasing values.
- HandleInvokeOrClInit(mir);
+ HandleInvokeOrClInitOrAcquireOp(mir);
}
uint16_t type = opcode - Instruction::SPUT;
if (!field_info.IsResolved()) {
@@ -1351,7 +1354,7 @@
}
}
-void LocalValueNumbering::HandleInvokeOrClInit(MIR* mir) {
+void LocalValueNumbering::HandleInvokeOrClInitOrAcquireOp(MIR* mir) {
// Use mir->offset as modifier; without elaborate inlining, it will be unique.
global_memory_version_ =
gvn_->LookupValue(kInvokeMemoryVersionBumpOp, 0u, 0u, mir->offset);
@@ -1404,9 +1407,7 @@
case Instruction::MONITOR_ENTER:
HandleNullCheck(mir, GetOperandValue(mir->ssa_rep->uses[0]));
- // NOTE: Keeping all aliasing values intact. Programs that rely on loads/stores of the
- // same non-volatile locations outside and inside a synchronized block being different
- // contain races that we cannot fix.
+ HandleInvokeOrClInitOrAcquireOp(mir); // Acquire operation.
break;
case Instruction::MONITOR_EXIT:
@@ -1468,7 +1469,7 @@
uint16_t reg = GetOperandValue(mir->ssa_rep->uses[i]);
non_aliasing_refs_.erase(reg);
}
- HandleInvokeOrClInit(mir);
+ HandleInvokeOrClInitOrAcquireOp(mir);
}
break;