Bug fixes for JIT loop detection and formation

Specifically:
- Don't apply loop optimization if the basic induction variable is
  manipulated (ie excluding cases like "i+=0")
- Fix a case where variables reloaded with constants in the body are
  not considered as loop invariants

Bug: 2804188
Change-Id: Ia5ebb29bc6814b1be069e23794585f8313900b7d
diff --git a/vm/compiler/Compiler.h b/vm/compiler/Compiler.h
index 1d074f2..9aeb661 100644
--- a/vm/compiler/Compiler.h
+++ b/vm/compiler/Compiler.h
@@ -225,6 +225,13 @@
 #define METHOD_IS_GETTER        (1 << kIsGetter)
 #define METHOD_IS_SETTER        (1 << kIsSetter)
 
+/* Vectors to provide optimization hints */
+typedef enum JitOptimizationHints {
+    kJitOptNoLoop = 0,          // Disable loop formation/optimization
+} JitOptimizationHints;
+
+#define JIT_OPT_NO_LOOP         (1 << kJitOptNoLoop)
+
 typedef struct CompilerMethodStats {
     const Method *method;       // Used as hash entry signature
     int dalvikSize;             // # of bytes for dalvik bytecodes
@@ -254,7 +261,7 @@
 bool dvmCompileMethod(struct CompilationUnit *cUnit, const Method *method,
                       JitTranslationInfo *info);
 bool dvmCompileTrace(JitTraceDescription *trace, int numMaxInsts,
-                     JitTranslationInfo *info, jmp_buf *bailPtr);
+                     JitTranslationInfo *info, jmp_buf *bailPtr, int optHints);
 void dvmCompilerDumpStats(void);
 void dvmCompilerDrainQueue(void);
 void dvmJitUnchainAll(void);
@@ -263,7 +270,7 @@
 void dvmCompilerInlineMIR(struct CompilationUnit *cUnit);
 void dvmInitializeSSAConversion(struct CompilationUnit *cUnit);
 int dvmConvertSSARegToDalvik(struct CompilationUnit *cUnit, int ssaReg);
-void dvmCompilerLoopOpt(struct CompilationUnit *cUnit);
+bool dvmCompilerLoopOpt(struct CompilationUnit *cUnit);
 void dvmCompilerNonLoopAnalysis(struct CompilationUnit *cUnit);
 void dvmCompilerFindLiveIn(struct CompilationUnit *cUnit,
                            struct BasicBlock *bb);
diff --git a/vm/compiler/Frontend.c b/vm/compiler/Frontend.c
index 40da0e1..a828886 100644
--- a/vm/compiler/Frontend.c
+++ b/vm/compiler/Frontend.c
@@ -399,7 +399,8 @@
  * bytecode into machine code.
  */
 bool dvmCompileTrace(JitTraceDescription *desc, int numMaxInsts,
-                     JitTranslationInfo *info, jmp_buf *bailPtr)
+                     JitTranslationInfo *info, jmp_buf *bailPtr,
+                     int optHints)
 {
     const DexCode *dexCode = dvmGetMethodCode(desc->method);
     const JitTraceRun* currRun = &desc->trace[0];
@@ -671,10 +672,12 @@
                        kInstrInvoke)) == 0) ||
             (lastInsn->dalvikInsn.opCode == OP_INVOKE_DIRECT_EMPTY);
 
+        /* Only form a loop if JIT_OPT_NO_LOOP is not set */
         if (curBB->taken == NULL &&
             curBB->fallThrough == NULL &&
             flags == (kInstrCanBranch | kInstrCanContinue) &&
-            fallThroughOffset == startBB->startOffset) {
+            fallThroughOffset == startBB->startOffset &&
+            JIT_OPT_NO_LOOP != (optHints & JIT_OPT_NO_LOOP)) {
             BasicBlock *loopBranch = curBB;
             BasicBlock *exitBB;
             BasicBlock *exitChainingCell;
@@ -883,7 +886,21 @@
     dvmInitializeSSAConversion(&cUnit);
 
     if (cUnit.hasLoop) {
-        dvmCompilerLoopOpt(&cUnit);
+        /*
+         * Loop is not optimizable (for example lack of a single induction
+         * variable), punt and recompile the trace with loop optimization
+         * disabled.
+         */
+        bool loopOpt = dvmCompilerLoopOpt(&cUnit);
+        if (loopOpt == false) {
+            if (cUnit.printMe) {
+                LOGD("Loop is not optimizable - retry codegen");
+            }
+            /* Reset the compiler resource pool */
+            dvmCompilerArenaReset();
+            return dvmCompileTrace(desc, cUnit.numInsts, info, bailPtr,
+                                   optHints | JIT_OPT_NO_LOOP);
+        }
     }
     else {
         dvmCompilerNonLoopAnalysis(&cUnit);
@@ -927,7 +944,8 @@
 
     /* Halve the instruction count and retry again */
     } else {
-        return dvmCompileTrace(desc, cUnit.numInsts / 2, info, bailPtr);
+        return dvmCompileTrace(desc, cUnit.numInsts / 2, info, bailPtr,
+                               optHints);
     }
 }
 
diff --git a/vm/compiler/Loop.c b/vm/compiler/Loop.c
index dede0ee..78f5717 100644
--- a/vm/compiler/Loop.c
+++ b/vm/compiler/Loop.c
@@ -159,6 +159,8 @@
  * 2) The loop back branch compares the BIV with a constant
  * 3) If it is a count-up loop, the condition is GE/GT, or LE/LT/LEZ/LTZ for
  *    a count-down loop.
+ *
+ * Return false if the loop is not optimizable.
  */
 static bool isLoopOptimizable(CompilationUnit *cUnit)
 {
@@ -173,6 +175,10 @@
         ivInfo = GET_ELEM_N(loopAnalysis->ivList, InductionVariableInfo*, i);
         /* Count up or down loop? */
         if (ivInfo->ssaReg == ivInfo->basicSSAReg) {
+            /* Infinite loop */
+            if (ivInfo->inc == 0) {
+                return false;
+            }
             loopAnalysis->isCountUpLoop = ivInfo->inc > 0;
             break;
         }
@@ -207,11 +213,14 @@
         }
         /*
          * If the comparison is not between the BIV and a loop invariant,
-         * return false.
+         * return false. endReg is loop invariant if one of the following is
+         * true:
+         * - It is not defined in the loop (ie DECODE_SUB returns 0)
+         * - It is reloaded with a constant
          */
         int endReg = dvmConvertSSARegToDalvik(cUnit, branch->ssaRep->uses[1]);
-
-        if (DECODE_SUB(endReg) != 0) {
+        if (DECODE_SUB(endReg) != 0 &&
+            !dvmIsBitSet(cUnit->isConstantV, branch->ssaRep->uses[1])) {
             return false;
         }
         loopAnalysis->endConditionReg = DECODE_REG(endReg);
@@ -457,8 +466,11 @@
     }
 }
 
-/* Main entry point to do loop optimization */
-void dvmCompilerLoopOpt(CompilationUnit *cUnit)
+/*
+ * Main entry point to do loop optimization.
+ * Return false if sanity checks for loop formation/optimization failed.
+ */
+bool dvmCompilerLoopOpt(CompilationUnit *cUnit)
 {
     LoopAnalysis *loopAnalysis = dvmCompilerNew(sizeof(LoopAnalysis), true);
 
@@ -497,7 +509,7 @@
 
     /* If the loop turns out to be non-optimizable, return early */
     if (!isLoopOptimizable(cUnit))
-        return;
+        return false;
 
     loopAnalysis->arrayAccessInfo = dvmCompilerNew(sizeof(GrowableList), true);
     dvmInitGrowableList(loopAnalysis->arrayAccessInfo, 4);
@@ -509,4 +521,5 @@
      * header.
      */
     genHoistedChecks(cUnit);
+    return true;
 }
diff --git a/vm/compiler/codegen/arm/CodegenDriver.c b/vm/compiler/codegen/arm/CodegenDriver.c
index a1e5449..dcabd8e 100644
--- a/vm/compiler/codegen/arm/CodegenDriver.c
+++ b/vm/compiler/codegen/arm/CodegenDriver.c
@@ -4339,14 +4339,14 @@
         case kWorkOrderTrace:
             /* Start compilation with maximally allowed trace length */
             res = dvmCompileTrace(work->info, JIT_MAX_TRACE_LEN, &work->result,
-                                  work->bailPtr);
+                                  work->bailPtr, 0 /* no hints */);
             break;
         case kWorkOrderTraceDebug: {
             bool oldPrintMe = gDvmJit.printMe;
             gDvmJit.printMe = true;
             /* Start compilation with maximally allowed trace length */
             res = dvmCompileTrace(work->info, JIT_MAX_TRACE_LEN, &work->result,
-                                  work->bailPtr);
+                                  work->bailPtr, 0 /* no hints */);
             gDvmJit.printMe = oldPrintMe;
             break;
         }