Try/Catch analysis; various workarounds
Fixed a couple of codegen bugs. Added a temporary workaround until
SSA renaming problem is fixed. By enabling the "CompileDexLibCore"
test in compiler_test.cc and disabling the jni_compiler, we appear to
be successfully compiling 17,641 methods of libcore (note: of those,
4 exhibit the SSA problem).
Also turned off most of the compiler logging, and disabled the fast
path for invoke virtual (which seems to be broken).
Change-Id: I0ecf460cba209f885209efbee62e9f80bffbf666
diff --git a/src/compiler/codegen/Ralloc.h b/src/compiler/codegen/Ralloc.h
index b73ae30..c702204 100644
--- a/src/compiler/codegen/Ralloc.h
+++ b/src/compiler/codegen/Ralloc.h
@@ -132,6 +132,7 @@
int low, int high);
// Get the LocRecord associated with an SSA name use.
extern RegLocation oatGetSrc(CompilationUnit* cUnit, MIR* mir, int num);
+extern RegLocation oatGetRawSrc(CompilationUnit* cUnit, MIR* mir, int num);
// Get the LocRecord associated with an SSA name def.
extern RegLocation oatGetDest(CompilationUnit* cUnit, MIR* mir, int num);
diff --git a/src/compiler/codegen/RallocUtil.cc b/src/compiler/codegen/RallocUtil.cc
index 6a9777e..d6e0bbc 100644
--- a/src/compiler/codegen/RallocUtil.cc
+++ b/src/compiler/codegen/RallocUtil.cc
@@ -1000,24 +1000,56 @@
return loc;
}
+/*
+ * There's currently a problem in SSA renaming. So long as register promotion
+ * is disabled, a bad renaming will have no effect. Work around the problem
+ * here to make progress while the fix is being identified.
+ */
+#define SSA_WORKAROUND
+
extern RegLocation oatGetDest(CompilationUnit* cUnit, MIR* mir, int num)
{
- return cUnit->regLocation[mir->ssaRep->defs[num]];
+ RegLocation res = cUnit->regLocation[mir->ssaRep->defs[num]];
+#ifdef SSA_WORKAROUND
+ res.wide = false;
+#endif
+ assert(!res.wide);
+ return res;
}
extern RegLocation oatGetSrc(CompilationUnit* cUnit, MIR* mir, int num)
{
- return cUnit->regLocation[mir->ssaRep->uses[num]];
+ RegLocation res = cUnit->regLocation[mir->ssaRep->uses[num]];
+#ifdef SSA_WORKAROUND
+ res.wide = false;
+#endif
+ assert(!res.wide);
+ return res;
+}
+extern RegLocation oatGetRawSrc(CompilationUnit* cUnit, MIR* mir, int num)
+{
+ RegLocation res = cUnit->regLocation[mir->ssaRep->uses[num]];
+ return res;
}
extern RegLocation oatGetDestWide(CompilationUnit* cUnit, MIR* mir,
int low, int high)
{
- return oatGetDest(cUnit, mir, low);
+ RegLocation res = cUnit->regLocation[mir->ssaRep->defs[low]];
+#ifdef SSA_WORKAROUND
+ res.wide = true;
+#endif
+ assert(res.wide);
+ return res;
}
extern RegLocation oatGetSrcWide(CompilationUnit* cUnit, MIR* mir,
int low, int high)
{
- return oatGetSrc(cUnit, mir, low);
+ RegLocation res = cUnit->regLocation[mir->ssaRep->uses[low]];
+#ifdef SSA_WORKAROUND
+ res.wide = true;
+#endif
+ assert(res.wide);
+ return res;
}
/* Kill the corresponding bit in the null-checked register list */
diff --git a/src/compiler/codegen/arm/ArchFactory.cc b/src/compiler/codegen/arm/ArchFactory.cc
index 2438719..574bb57 100644
--- a/src/compiler/codegen/arm/ArchFactory.cc
+++ b/src/compiler/codegen/arm/ArchFactory.cc
@@ -78,7 +78,10 @@
int dOffset, TGT_LIR* pcrLabel)
{
/* This particular Dalvik register has been null-checked */
+#if 0
+ // Yes, I know. Please be quiet.
UNIMPLEMENTED(WARNING) << "Need null check & throw support";
+#endif
return pcrLabel;
if (oatIsBitSet(cUnit->regPool->nullCheckedRegs, sReg)) {
return pcrLabel;
diff --git a/src/compiler/codegen/arm/ArmRallocUtil.cc b/src/compiler/codegen/arm/ArmRallocUtil.cc
index 78d5267..bea85ea 100644
--- a/src/compiler/codegen/arm/ArmRallocUtil.cc
+++ b/src/compiler/codegen/arm/ArmRallocUtil.cc
@@ -35,10 +35,7 @@
bool doubleStart; // Starting vReg for a double
} RefCounts;
-/*
- * USE SSA names to count references of base Dalvik vRegs. Also,
- * mark "wide" in the first of wide SSA locationRec pairs.
- */
+/* USE SSA names to count references of base Dalvik vRegs. */
static void countRefs(CompilationUnit *cUnit, BasicBlock* bb,
RefCounts* counts, bool fp)
{
@@ -61,16 +58,12 @@
oatConvertSSARegToDalvik(cUnit, ssaRep->defs[0]));
counts[sReg].doubleStart = true;
}
- if (attrs & DF_DA_WIDE) {
- cUnit->regLocation[ssaRep->defs[0]].wide = true;
- }
if ((attrs & (DF_UA_WIDE|DF_FP_A)) == (DF_UA_WIDE|DF_FP_A)) {
sReg = DECODE_REG(
oatConvertSSARegToDalvik(cUnit, ssaRep->uses[first]));
counts[sReg].doubleStart = true;
}
if (attrs & DF_UA_WIDE) {
- cUnit->regLocation[ssaRep->uses[first]].wide = true;
first += 2;
}
if ((attrs & (DF_UB_WIDE|DF_FP_B)) == (DF_UB_WIDE|DF_FP_B)) {
@@ -79,7 +72,6 @@
counts[sReg].doubleStart = true;
}
if (attrs & DF_UB_WIDE) {
- cUnit->regLocation[ssaRep->uses[first]].wide = true;
first += 2;
}
if ((attrs & (DF_UC_WIDE|DF_FP_C)) == (DF_UC_WIDE|DF_FP_C)) {
@@ -87,9 +79,6 @@
oatConvertSSARegToDalvik(cUnit, ssaRep->uses[first]));
counts[sReg].doubleStart = true;
}
- if (attrs & DF_UC_WIDE) {
- cUnit->regLocation[ssaRep->uses[first]].wide = true;
- }
}
for (i=0; i< ssaRep->numUses; i++) {
int origSreg = DECODE_REG(
@@ -140,27 +129,8 @@
extern void oatDoPromotion(CompilationUnit* cUnit)
{
int numRegs = cUnit->method->NumRegisters();
- int numIns = cUnit->method->NumIns();
/*
- * Because ins don't have explicit definitions, we need to type
- * them based on the signature.
- */
- if (numIns > 0) {
- int sReg = numRegs - numIns;
- const art::StringPiece& shorty = cUnit->method->GetShorty();
- for (int i = 1; i < shorty.size(); i++) {
- char arg = shorty[i];
- // Is it wide?
- if ((arg == 'D') || (arg == 'J')) {
- cUnit->regLocation[sReg].wide = true;
- cUnit->regLocation[sReg+1].fp = cUnit->regLocation[sReg].fp;
- sReg++; // Skip to next
- }
- sReg++;
- }
- }
- /*
* TUNING: is leaf? Can't just use "hasInvoke" to determine as some
* instructions might call out to C/assembly helper functions. Until
* machinery is in place, always spill lr.
diff --git a/src/compiler/codegen/arm/MethodCodegenDriver.cc b/src/compiler/codegen/arm/MethodCodegenDriver.cc
index b5ce6ea..13e5497 100644
--- a/src/compiler/codegen/arm/MethodCodegenDriver.cc
+++ b/src/compiler/codegen/arm/MethodCodegenDriver.cc
@@ -14,6 +14,8 @@
* limitations under the License.
*/
+#define FORCE_SLOW 1
+
static const RegLocation badLoc = {kLocDalvikFrame, 0, 0, INVALID_REG,
INVALID_REG, INVALID_SREG, 0,
kLocDalvikFrame, INVALID_REG, INVALID_REG,
@@ -200,7 +202,7 @@
{
int fieldIdx = mir->dalvikInsn.vB;
Field* field = cUnit->method->GetDexCacheResolvedFields()->Get(fieldIdx);
- if (field == NULL) {
+ if (FORCE_SLOW || field == NULL) {
oatFlushAllRegs(cUnit);
loadWordDisp(cUnit, rSELF, OFFSETOF_MEMBER(Thread, pSet64Static), rLR);
loadConstant(cUnit, r0, mir->dalvikInsn.vB);
@@ -256,7 +258,7 @@
{
int fieldIdx = mir->dalvikInsn.vB;
Field* field = cUnit->method->GetDexCacheResolvedFields()->Get(fieldIdx);
- if (field == NULL) {
+ if (FORCE_SLOW || field == NULL) {
oatFlushAllRegs(cUnit);
loadWordDisp(cUnit, rSELF, OFFSETOF_MEMBER(Thread, pGet64Static), rLR);
loadConstant(cUnit, r0, mir->dalvikInsn.vB);
@@ -314,7 +316,7 @@
Field* field = cUnit->method->GetDexCacheResolvedFields()->Get(fieldIdx);
bool isObject = ((mir->dalvikInsn.opcode == OP_SGET_OBJECT) ||
(mir->dalvikInsn.opcode == OP_SGET_OBJECT_VOLATILE));
- if (field == NULL) {
+ if (FORCE_SLOW || field == NULL) {
// Slow path
int funcOffset = isObject ? OFFSETOF_MEMBER(Thread, pGetObjStatic)
: OFFSETOF_MEMBER(Thread, pGet32Static);
@@ -527,8 +529,8 @@
{
for (int i = 0; i < 3; i++) {
if (args[i] != INVALID_REG) {
- RegLocation rlArg = oatGetSrc(cUnit, mir, i);
// Arguments are treated as a series of untyped 32-bit values.
+ RegLocation rlArg = oatGetRawSrc(cUnit, mir, i);
rlArg.wide = false;
loadValueDirectFixed(cUnit, rlArg, r1 + i);
callState = nextCallInsn(cUnit, mir, dInsn, callState, rollback);
@@ -709,6 +711,8 @@
RegLocation rlArg;
int registerArgs[3];
+skipThis = false;
+
/* If no arguments, just return */
if (dInsn->vA == 0)
return callState;
@@ -722,8 +726,7 @@
*/
for (unsigned int i=3; i < dInsn->vA; i++) {
int reg;
- int arg = (isRange) ? dInsn->vC + i : i;
- rlArg = oatUpdateLoc(cUnit, oatGetSrc(cUnit, mir, arg));
+ rlArg = oatUpdateLoc(cUnit, oatGetSrc(cUnit, mir, i));
if (rlArg.location == kLocPhysReg) {
reg = rlArg.lowReg;
} else {
@@ -736,12 +739,15 @@
}
/* Load register arguments r1..r3 */
- for (unsigned int i = skipThis ? 1 : 0; i < 3; i++) {
+ for (unsigned int i = 0; i < 3; i++) {
if (i < dInsn->vA)
registerArgs[i] = (isRange) ? dInsn->vC + i : i;
else
registerArgs[i] = INVALID_REG;
}
+ if (skipThis) {
+ registerArgs[0] = INVALID_REG;
+ }
callState = loadArgRegs(cUnit, mir, dInsn, callState, registerArgs,
nextCallInsn, rollback);
@@ -775,6 +781,9 @@
{
int firstArg = dInsn->vC;
int numArgs = dInsn->vA;
+ int registerArgs[3];
+
+skipThis = false;
// If we can treat it as non-range (Jumbo ops will use range form)
if (numArgs <= 5)
@@ -798,7 +807,7 @@
*/
// Scan the rest of the args - if in physReg flush to memory
for (int i = 4; i < numArgs; i++) {
- RegLocation loc = oatGetSrc(cUnit, mir, i);
+ RegLocation loc = oatGetRawSrc(cUnit, mir, i);
if (loc.wide) {
loc = oatUpdateLocWide(cUnit, loc);
if (loc.location == kLocPhysReg) { // TUNING: if dirty?
@@ -840,11 +849,17 @@
}
// Handle the 1st 3 in r1, r2 & r3
- for (unsigned int i = skipThis? 1 : 0; i < dInsn->vA && i < 3; i++) {
- RegLocation loc = oatGetSrc(cUnit, mir, firstArg + i);
- loadValueDirectFixed(cUnit, loc, r1 + i);
- callState = nextCallInsn(cUnit, mir, dInsn, callState, rollback);
+ for (unsigned int i = 0; i < 3; i++) {
+ if (i < dInsn->vA)
+ registerArgs[i] = dInsn->vC + i;
+ else
+ registerArgs[i] = INVALID_REG;
}
+ if (skipThis) {
+ registerArgs[0] = INVALID_REG;
+ }
+ callState = loadArgRegs(cUnit, mir, dInsn, callState, registerArgs,
+ nextCallInsn, rollback);
// Finally, deal with the register arguments
// We'll be using fixed registers here
@@ -958,9 +973,7 @@
Get(dInsn->vB);
NextCallInsn nextCallInsn;
- method = NULL; // TODO
- UNIMPLEMENTED(WARNING) << "the genInvokeVirtual fast path generates bad code (r0/r9 mixup?)";
- if (method == NULL) {
+ if (FORCE_SLOW || method == NULL) {
// Slow path
nextCallInsn = nextVCallInsnSP;
// If we need a slow-path callout, we'll restart here
diff --git a/src/compiler/codegen/arm/Thumb2/Gen.cc b/src/compiler/codegen/arm/Thumb2/Gen.cc
index 9ea9e70..b9e135c 100644
--- a/src/compiler/codegen/arm/Thumb2/Gen.cc
+++ b/src/compiler/codegen/arm/Thumb2/Gen.cc
@@ -1232,6 +1232,7 @@
/* regPtr -> array data */
opRegRegImm(cUnit, kOpAdd, regPtr, rlArray.lowReg, dataOffset);
}
+ oatFreeTemp(cUnit, rlArray.lowReg);
if ((size == kLong) || (size == kDouble)) {
if (scale) {
int rNewIndex = oatAllocTemp(cUnit);
@@ -1241,6 +1242,7 @@
} else {
opRegReg(cUnit, kOpAdd, regPtr, rlIndex.lowReg);
}
+ oatFreeTemp(cUnit, rlIndex.lowReg);
rlResult = oatEvalLoc(cUnit, rlDest, regClass, true);
loadPair(cUnit, regPtr, rlResult.lowReg, rlResult.highReg);