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

1 files changed, 14 insertions, 20 deletions

diff --git a/src/compiler/codegen/GenCommon.cc b/src/compiler/codegen/GenCommon.cc
index 6792f61e2e..3d46240df6 100644
--- a/src/compiler/codegen/GenCommon.cc
+++ b/src/compiler/codegen/GenCommon.cc
@@ -607,25 +607,13 @@ void handleSuspendLaunchpads(CompilationUnit *cUnit)
 
     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 @@ void genConstString(CompilationUnit* cUnit, MIR* mir, RegLocation rlDest,
         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 @@ void genSuspendTest(CompilationUnit* cUnit, MIR* mir)
         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 @@ void genSuspendTest(CompilationUnit* cUnit, MIR* mir)
         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
 }