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/MethodCodegenDriver.cc b/src/compiler/codegen/MethodCodegenDriver.cc
index 8dd099e..db491f1 100644
--- a/src/compiler/codegen/MethodCodegenDriver.cc
+++ b/src/compiler/codegen/MethodCodegenDriver.cc
@@ -25,18 +25,21 @@
INVALID_REG, INVALID_REG, INVALID_SREG};
/* Mark register usage state and return long retloc */
-RegLocation getRetLocWide(CompilationUnit* cUnit)
+RegLocation oatGetReturnWide(CompilationUnit* cUnit)
{
RegLocation res = LOC_C_RETURN_WIDE;
+ oatClobber(cUnit, res.lowReg);
+ oatClobber(cUnit, res.highReg);
oatLockTemp(cUnit, res.lowReg);
oatLockTemp(cUnit, res.highReg);
oatMarkPair(cUnit, res.lowReg, res.highReg);
return res;
}
-RegLocation getRetLoc(CompilationUnit* cUnit)
+RegLocation oatGetReturn(CompilationUnit* cUnit)
{
RegLocation res = LOC_C_RETURN;
+ oatClobber(cUnit, res.lowReg);
oatLockTemp(cUnit, res.lowReg);
return res;
}
@@ -178,31 +181,37 @@
break;
case Instruction::RETURN_VOID:
- genSuspendTest(cUnit, mir);
+ if (!cUnit->attrs & METHOD_IS_LEAF) {
+ genSuspendTest(cUnit, mir);
+ }
break;
case Instruction::RETURN:
case Instruction::RETURN_OBJECT:
- genSuspendTest(cUnit, mir);
- storeValue(cUnit, getRetLoc(cUnit), rlSrc[0]);
+ if (!cUnit->attrs & METHOD_IS_LEAF) {
+ genSuspendTest(cUnit, mir);
+ }
+ storeValue(cUnit, oatGetReturn(cUnit), rlSrc[0]);
break;
case Instruction::RETURN_WIDE:
- genSuspendTest(cUnit, mir);
- storeValueWide(cUnit, getRetLocWide(cUnit), rlSrc[0]);
+ if (!cUnit->attrs & METHOD_IS_LEAF) {
+ genSuspendTest(cUnit, mir);
+ }
+ storeValueWide(cUnit, oatGetReturnWide(cUnit), rlSrc[0]);
break;
case Instruction::MOVE_RESULT_WIDE:
if (mir->optimizationFlags & MIR_INLINED)
break; // Nop - combined w/ previous invoke
- storeValueWide(cUnit, rlDest, getRetLocWide(cUnit));
+ storeValueWide(cUnit, rlDest, oatGetReturnWide(cUnit));
break;
case Instruction::MOVE_RESULT:
case Instruction::MOVE_RESULT_OBJECT:
if (mir->optimizationFlags & MIR_INLINED)
break; // Nop - combined w/ previous invoke
- storeValue(cUnit, rlDest, getRetLoc(cUnit));
+ storeValue(cUnit, rlDest, oatGetReturn(cUnit));
break;
case Instruction::MOVE:
@@ -849,7 +858,9 @@
handleThrowLaunchpads(cUnit);
- removeRedundantBranches(cUnit);
+ if (!(cUnit->disableOpt & (1 << kSafeOptimizations))) {
+ removeRedundantBranches(cUnit);
+ }
}
/* Needed by the ld/st optmizatons */