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/GenInvoke.cc b/src/compiler/codegen/GenInvoke.cc
index 4698868..c2d0d18 100644
--- a/src/compiler/codegen/GenInvoke.cc
+++ b/src/compiler/codegen/GenInvoke.cc
@@ -43,38 +43,52 @@
int lastArgReg = rARG3;
int startVReg = cUnit->numDalvikRegisters - cUnit->numIns;
/*
- * Arguments passed in registers should be flushed
- * to their backing locations in the frame for now.
- * Also, we need to do initial assignment for promoted
- * arguments. NOTE: an older version of dx had an issue
- * in which it would reuse static method argument registers.
+ * Copy incoming arguments to their proper home locations.
+ * NOTE: an older version of dx had an issue in which
+ * it would reuse static method argument registers.
* This could result in the same Dalvik virtual register
- * being promoted to both core and fp regs. In those
- * cases, copy argument to both. This will be uncommon
- * enough that it isn't worth attempting to optimize.
+ * being promoted to both core and fp regs. To account for this,
+ * we only copy to the corresponding promoted physical register
+ * if it matches the type of the SSA name for the incoming
+ * argument. It is also possible that long and double arguments
+ * end up half-promoted. In those cases, we must flush the promoted
+ * half to memory as well.
*/
for (int i = 0; i < cUnit->numIns; i++) {
- PromotionMap vMap = cUnit->promotionMap[startVReg + i];
+ PromotionMap* vMap = &cUnit->promotionMap[startVReg + i];
if (i <= (lastArgReg - firstArgReg)) {
// If arriving in register
- if (vMap.coreLocation == kLocPhysReg) {
- opRegCopy(cUnit, vMap.coreReg, firstArgReg + i);
+ bool needFlush = true;
+ RegLocation* tLoc = &cUnit->regLocation[startVReg + i];
+ if ((vMap->coreLocation == kLocPhysReg) && !tLoc->fp) {
+ opRegCopy(cUnit, vMap->coreReg, firstArgReg + i);
+ needFlush = false;
+ } else if ((vMap->fpLocation == kLocPhysReg) && tLoc->fp) {
+ opRegCopy(cUnit, vMap->fpReg, firstArgReg + i);
+ needFlush = false;
+ } else {
+ needFlush = true;
}
- if (vMap.fpLocation == kLocPhysReg) {
- opRegCopy(cUnit, vMap.fpReg, firstArgReg + i);
+
+ // For wide args, force flush if only half is promoted
+ if (tLoc->wide) {
+ PromotionMap* pMap = vMap + (tLoc->highWord ? -1 : +1);
+ needFlush |= (pMap->coreLocation != vMap->coreLocation) ||
+ (pMap->fpLocation != vMap->fpLocation);
}
- // Also put a copy in memory in case we're partially promoted
- storeBaseDisp(cUnit, rSP, oatSRegOffset(cUnit, startVReg + i),
- firstArgReg + i, kWord);
+ if (needFlush) {
+ storeBaseDisp(cUnit, rSP, oatSRegOffset(cUnit, startVReg + i),
+ firstArgReg + i, kWord);
+ }
} else {
// If arriving in frame & promoted
- if (vMap.coreLocation == kLocPhysReg) {
+ if (vMap->coreLocation == kLocPhysReg) {
loadWordDisp(cUnit, rSP, oatSRegOffset(cUnit, startVReg + i),
- vMap.coreReg);
+ vMap->coreReg);
}
- if (vMap.fpLocation == kLocPhysReg) {
+ if (vMap->fpLocation == kLocPhysReg) {
loadWordDisp(cUnit, rSP, oatSRegOffset(cUnit, startVReg + i),
- vMap.fpReg);
+ vMap->fpReg);
}
}
}