From 86a4bce32e2aaf3d377c0acf865f0630a7c30495 Mon Sep 17 00:00:00 2001
From: buzbee <buzbee@google.com>
Date: Tue, 6 Mar 2012 18:15:00 -0800
Subject: 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
---
 src/compiler/codegen/MethodCodegenDriver.cc | 31 +++++++++++++++++++----------
 1 file changed, 21 insertions(+), 10 deletions(-)

(limited to 'src/compiler/codegen/MethodCodegenDriver.cc')

diff --git a/src/compiler/codegen/MethodCodegenDriver.cc b/src/compiler/codegen/MethodCodegenDriver.cc
index 8dd099e598..db491f1836 100644
--- a/src/compiler/codegen/MethodCodegenDriver.cc
+++ b/src/compiler/codegen/MethodCodegenDriver.cc
@@ -25,18 +25,21 @@ const RegLocation badLoc = {kLocDalvikFrame, 0, 0, 0, 0, 0, 0,
                             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 @@ bool compileDalvikInstruction(CompilationUnit* cUnit, MIR* mir,
             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 @@ void oatMethodMIR2LIR(CompilationUnit* cUnit)
 
     handleThrowLaunchpads(cUnit);
 
-    removeRedundantBranches(cUnit);
+    if (!(cUnit->disableOpt & (1 << kSafeOptimizations))) {
+        removeRedundantBranches(cUnit);
+    }
 }
 
 /* Needed by the ld/st optmizatons */
-- 
cgit v1.2.3-59-g8ed1b