Add memory barriers (Issue 3338450)
Add missing barriers required for volatile access on Arm. Also,
flipped long volatile field accesses to the slow path. The
previous codegen was not only missing barriers, but was not
using 64-bit atomics.
Change-Id: I4aa2be2bf81971e5ae664c762ceaf2ea58ce231b
diff --git a/src/compiler/codegen/arm/ArmLIR.h b/src/compiler/codegen/arm/ArmLIR.h
index 20fa0a2..8f71571 100644
--- a/src/compiler/codegen/arm/ArmLIR.h
+++ b/src/compiler/codegen/arm/ArmLIR.h
@@ -840,7 +840,7 @@
struct {
bool isNop:1; // LIR is optimized away
bool insertWrapper:1; // insert branch to emulate memory accesses
- bool squashed:1; // Elminated def
+ bool squashed:1; // Eliminated def
unsigned int age:4; // default is 0, set lazily by the optimizer
unsigned int size:3; // bytes (2 for thumb, 2/4 for thumb2)
unsigned int unused:22;
diff --git a/src/compiler/codegen/arm/MethodCodegenDriver.cc b/src/compiler/codegen/arm/MethodCodegenDriver.cc
index cfeb16a..9e026be 100644
--- a/src/compiler/codegen/arm/MethodCodegenDriver.cc
+++ b/src/compiler/codegen/arm/MethodCodegenDriver.cc
@@ -222,6 +222,11 @@
branchOver->generic.target = (LIR*)skipTarget;
rlSrc = oatGetSrc(cUnit, mir, 0);
rlSrc = loadValue(cUnit, rlSrc, kAnyReg);
+#if ANDROID_SMP != 0
+ if (field->IsVolatile()) {
+ oatGenMemBarrier(cUnit, kST);
+ }
+#endif
storeWordDisp(cUnit, rBase, fieldOffset, rlSrc.lowReg);
#if ANDROID_SMP != 0
if (field->IsVolatile()) {
@@ -241,7 +246,12 @@
uint32_t typeIdx;
Field* field = FindFieldWithResolvedStaticStorage(cUnit->method, fieldIdx, typeIdx);
oatFlushAllRegs(cUnit);
- if (SLOW_FIELD_PATH || field == NULL) {
+#if ANDROID_SMP != 0
+ bool isVolatile = (field == NULL) || field->IsVolatile();
+#else
+ bool isVolatile = false;
+#endif
+ if (SLOW_FIELD_PATH || field == NULL || isVolatile) {
LOG(INFO) << "Field " << fieldNameFromIndex(cUnit->method, fieldIdx)
<< " unresolved at compile time";
loadWordDisp(cUnit, rSELF, OFFSETOF_MEMBER(Thread, pSet64Static), rLR);
@@ -276,11 +286,6 @@
rlSrc = loadValueWide(cUnit, rlSrc, kAnyReg);
storeBaseDispWide(cUnit, rBase, fieldOffset, rlSrc.lowReg,
rlSrc.highReg);
-#if ANDROID_SMP != 0
- if (field->IsVolatile()) {
- oatGenMemBarrier(cUnit, kSY);
- }
-#endif
oatFreeTemp(cUnit, rBase);
}
}
@@ -292,8 +297,13 @@
int fieldIdx = mir->dalvikInsn.vB;
uint32_t typeIdx;
Field* field = FindFieldWithResolvedStaticStorage(cUnit->method, fieldIdx, typeIdx);
+#if ANDROID_SMP != 0
+ bool isVolatile = (field == NULL) || field->IsVolatile();
+#else
+ bool isVolatile = false;
+#endif
oatFlushAllRegs(cUnit);
- if (SLOW_FIELD_PATH || field == NULL) {
+ if (SLOW_FIELD_PATH || field == NULL || isVolatile) {
LOG(INFO) << "Field " << fieldNameFromIndex(cUnit->method, fieldIdx)
<< " unresolved at compile time";
loadWordDisp(cUnit, rSELF, OFFSETOF_MEMBER(Thread, pGet64Static), rLR);
@@ -327,11 +337,6 @@
branchOver->generic.target = (LIR*)skipTarget;
rlDest = oatGetDestWide(cUnit, mir, 0, 1);
RegLocation rlResult = oatEvalLoc(cUnit, rlDest, kAnyReg, true);
-#if ANDROID_SMP != 0
- if (field->IsVolatile()) {
- oatGenMemBarrier(cUnit, kSY);
- }
-#endif
loadBaseDispWide(cUnit, NULL, rBase, fieldOffset, rlResult.lowReg,
rlResult.highReg, INVALID_SREG);
oatFreeTemp(cUnit, rBase);
diff --git a/src/compiler/codegen/arm/Thumb2/Gen.cc b/src/compiler/codegen/arm/Thumb2/Gen.cc
index 5c83660..d81c200 100644
--- a/src/compiler/codegen/arm/Thumb2/Gen.cc
+++ b/src/compiler/codegen/arm/Thumb2/Gen.cc
@@ -500,9 +500,12 @@
genNullCheck(cUnit, rlObj.sRegLow, rlObj.lowReg, mir);/* null obj? */
if (isVolatile) {
- oatGenMemBarrier(cUnit, kSY);
+ oatGenMemBarrier(cUnit, kST);
}
storeBaseDisp(cUnit, rlObj.lowReg, fieldOffset, rlSrc.lowReg, size);
+ if (isVolatile) {
+ oatGenMemBarrier(cUnit, kSY);
+ }
}
if (isObject) {
/* NOTE: marking card based on object head */
@@ -513,10 +516,15 @@
STATIC void genIGetWide(CompilationUnit* cUnit, MIR* mir, RegLocation rlDest,
RegLocation rlObj)
{
+ RegLocation rlResult;
Field* fieldPtr = cUnit->method->GetDeclaringClass()->GetDexCache()->
GetResolvedField(mir->dalvikInsn.vC);
- RegLocation rlResult;
- if (fieldPtr == NULL) {
+#if ANDROID_SMP != 0
+ bool isVolatile = (fieldPtr == NULL) || fieldPtr->IsVolatile();
+#else
+ bool isVolatile = false;
+#endif
+ if ((fieldPtr == NULL) || isVolatile) {
getFieldOffset(cUnit, mir);
// Field offset in r0
rlObj = loadValue(cUnit, rlObj, kCoreReg);
@@ -525,13 +533,8 @@
opRegReg(cUnit, kOpAdd, r0, rlObj.lowReg);
loadPair(cUnit, r0, rlResult.lowReg, rlResult.highReg);
oatGenMemBarrier(cUnit, kSY);
- storeValue(cUnit, rlDest, rlResult);
+ storeValueWide(cUnit, rlDest, rlResult);
} else {
-#if ANDROID_SMP != 0
- bool isVolatile = fieldPtr->IsVolatile();
-#else
- bool isVolatile = false;
-#endif
int fieldOffset = fieldPtr->GetOffset().Int32Value();
rlObj = loadValue(cUnit, rlObj, kCoreReg);
int regPtr = oatAllocTemp(cUnit);
@@ -544,10 +547,6 @@
loadPair(cUnit, regPtr, rlResult.lowReg, rlResult.highReg);
- if (isVolatile) {
- oatGenMemBarrier(cUnit, kSY);
- }
-
oatFreeTemp(cUnit, regPtr);
storeValueWide(cUnit, rlDest, rlResult);
}
@@ -558,7 +557,12 @@
{
Field* fieldPtr = cUnit->method->GetDeclaringClass()->GetDexCache()->
GetResolvedField(mir->dalvikInsn.vC);
- if (fieldPtr == NULL) {
+#if ANDROID_SMP != 0
+ bool isVolatile = (fieldPtr == NULL) || fieldPtr->IsVolatile();
+#else
+ bool isVolatile = false;
+#endif
+ if ((fieldPtr == NULL) || isVolatile) {
getFieldOffset(cUnit, mir);
// Field offset in r0
rlObj = loadValue(cUnit, rlObj, kCoreReg);
@@ -568,11 +572,6 @@
oatGenMemBarrier(cUnit, kSY);
storePair(cUnit, r0, rlSrc.lowReg, rlSrc.highReg);
} else {
-#if ANDROID_SMP != 0
- bool isVolatile = fieldPtr->IsVolatile();
-#else
- bool isVolatile = false;
-#endif
int fieldOffset = fieldPtr->GetOffset().Int32Value();
rlObj = loadValue(cUnit, rlObj, kCoreReg);
@@ -582,9 +581,6 @@
regPtr = oatAllocTemp(cUnit);
opRegRegImm(cUnit, kOpAdd, regPtr, rlObj.lowReg, fieldOffset);
- if (isVolatile) {
- oatGenMemBarrier(cUnit, kSY);
- }
storePair(cUnit, regPtr, rlSrc.lowReg, rlSrc.highReg);
oatFreeTemp(cUnit, regPtr);