Fix branch bug (showed up in codegen for debug)
There are a few "safe" optimizations in the compiler - removing
register copies where source and target are the same, deleting
branches to the next instruction, etc. One of the redundant
branch optimizations, however, was incorrect and resulted in
a good branch being deleted. This one showed up in the debug
build, and resulted in a failure to do a suspend check (because
the branch to the suspend check was deleted).
I had hoped that this but might also be the case of some
other unexpected failures, but unfortunately I was only able
to trigger it when doing a "codegen for debug" build.
The source of the bug was a confusion around 16 v/ 32-bit
unconditional branch encodings. For a 32-bit unconditional
branch, going to the next instruction means an displacement
of zero. However, for 16-bit branches, the next instruction
is represented by a displacement of -1.
To help track down this sort of thing in the future, this CL
also adds a new optimization disable flag: kSafeOptimizations.
This will allow us to really turn off all optimizations for A/B
testing.
Also in this CL we are re-enabling the ability to promote argument
registers and improving somewhat the code sequence for suspend
check when debug is enabled.
Change-Id: Ib6b202746eac751cab3b4609805a389c18cb67b2
diff --git a/src/compiler/codegen/GenCommon.cc b/src/compiler/codegen/GenCommon.cc
index 6792f61..3d46240 100644
--- a/src/compiler/codegen/GenCommon.cc
+++ b/src/compiler/codegen/GenCommon.cc
@@ -607,25 +607,13 @@
for (int i = 0; i < numElems; i++) {
oatResetRegPool(cUnit);
- /* TUNING: move suspend count load into helper */
LIR* lab = suspendLabel[i];
LIR* resumeLab = (LIR*)lab->operands[0];
cUnit->currentDalvikOffset = lab->operands[1];
oatAppendLIR(cUnit, (LIR *)lab);
int rTgt = loadHelper(cUnit, OFFSETOF_MEMBER(Thread,
pTestSuspendFromCode));
- if (!cUnit->genDebugger) {
- // use rSUSPEND for suspend count
- loadWordDisp(cUnit, rSELF,
- Thread::SuspendCountOffset().Int32Value(), rSUSPEND);
- }
opReg(cUnit, kOpBlx, rTgt);
- if ( cUnit->genDebugger) {
- // use rSUSPEND for update debugger
- loadWordDisp(cUnit, rSELF,
- OFFSETOF_MEMBER(Thread, pUpdateDebuggerFromCode),
- rSUSPEND);
- }
opUnconditionalBranch(cUnit, resumeLab);
}
#endif
@@ -930,7 +918,7 @@
branch->target = target;
#endif
genBarrier(cUnit);
- storeValue(cUnit, rlDest, getRetLoc(cUnit));
+ storeValue(cUnit, rlDest, oatGetReturn(cUnit));
} else {
int mReg = loadCurrMethod(cUnit);
int resReg = oatAllocTemp(cUnit);
@@ -2137,11 +2125,17 @@
return;
}
oatFlushAllRegs(cUnit);
- LIR* branch;
if (cUnit->genDebugger) {
// If generating code for the debugger, always check for suspension
- branch = opUnconditionalBranch(cUnit, NULL);
+ int rTgt = loadHelper(cUnit, OFFSETOF_MEMBER(Thread,
+ pTestSuspendFromCode));
+ opReg(cUnit, kOpBlx, rTgt);
+ // Refresh rSUSPEND
+ loadWordDisp(cUnit, rSELF,
+ OFFSETOF_MEMBER(Thread, pUpdateDebuggerFromCode),
+ rSUSPEND);
} else {
+ LIR* branch;
#if defined(TARGET_ARM)
// In non-debug case, only check periodically
newLIR2(cUnit, kThumbSubRI8, rSUSPEND, 1);
@@ -2150,12 +2144,12 @@
opRegImm(cUnit, kOpSub, rSUSPEND, 1);
branch = opCmpImmBranch(cUnit, kCondEq, rSUSPEND, 0, NULL);
#endif
+ LIR* retLab = newLIR0(cUnit, kPseudoTargetLabel);
+ LIR* target = rawLIR(cUnit, cUnit->currentDalvikOffset,
+ kPseudoSuspendTarget, (intptr_t)retLab, mir->offset);
+ branch->target = (LIR*)target;
+ oatInsertGrowableList(cUnit, &cUnit->suspendLaunchpads, (intptr_t)target);
}
- LIR* retLab = newLIR0(cUnit, kPseudoTargetLabel);
- LIR* target = rawLIR(cUnit, cUnit->currentDalvikOffset,
- kPseudoSuspendTarget, (intptr_t)retLab, mir->offset);
- branch->target = (LIR*)target;
- oatInsertGrowableList(cUnit, &cUnit->suspendLaunchpads, (intptr_t)target);
#endif
}