Bug fix for control-flow construction involving nested loops.

Bug: 5884080
Change-Id: Ic2f42abae3e71f9c0729984902396f92902184a3

diff --git a/tests/083-jit-regressions/expected.txt b/tests/083-jit-regressions/expected.txt
index 1f30d21..4b9ad5b 100644 (file)
--- a/tests/083-jit-regressions/expected.txt
+++ b/tests/083-jit-regressions/expected.txt
@@ -1,3 +1,4 @@
 b2296099 passes
 b2302318 passes
 b2487514 passes
+b5884080 passes

diff --git a/tests/083-jit-regressions/info.txt b/tests/083-jit-regressions/info.txt
index b791aba..00c24ee 100644 (file)
--- a/tests/083-jit-regressions/info.txt
+++ b/tests/083-jit-regressions/info.txt
@@ -8,3 +8,4 @@ This test covers JIT regressions
 2296099 JIT shift bug
 2302318 Crash during spin-on-suspend testing
 2487514 Missed exception in PriorityBlockingQueueTest.testToArray1_BadArg
+5884080 ICS JIT regression in nested loop formation

diff --git a/tests/083-jit-regressions/src/Main.java b/tests/083-jit-regressions/src/Main.java
index 1f1dee3..3b596db 100644 (file)
--- a/tests/083-jit-regressions/src/Main.java
+++ b/tests/083-jit-regressions/src/Main.java
@@ -24,6 +24,7 @@ public class Main {
         b2296099Test();
         b2302318Test();
         b2487514Test();
+        b5884080Test();
     }
 
     static void b2296099Test() throws Exception {
@@ -105,6 +106,26 @@ public class Main {
                                " (expecting 1000)");
         }
     }
+
+    static void b5884080Test() {
+        int vA = 1;
+
+        int l = 0;
+        do
+        {
+            int k = 0;
+            do
+                vA += 1;
+            while(++k < 100);
+        } while(++l < 1000);
+        if (vA == 100001) {
+            System.out.println("b5884080 passes");
+        }
+        else {
+            System.out.println("b5884080 fails: vA is " + vA +
+                               " (expecting 100001)");
+        }
+    }
 }
 
 class SpinThread extends Thread {

diff --git a/vm/compiler/Frontend.cpp b/vm/compiler/Frontend.cpp
index 24ec6c7..723de86 100644 (file)
--- a/vm/compiler/Frontend.cpp
+++ b/vm/compiler/Frontend.cpp
@@ -536,7 +536,8 @@ bool dvmCompilerCanIncludeThisInstruction(const Method *method,
 /* Split an existing block from the specified code offset into two */
 static BasicBlock *splitBlock(CompilationUnit *cUnit,
                               unsigned int codeOffset,
-                              BasicBlock *origBlock)
+                              BasicBlock *origBlock,
+                              BasicBlock **immedPredBlockP)
 {
     MIR *insn = origBlock->firstMIRInsn;
     while (insn) {
@@ -598,16 +599,29 @@ static BasicBlock *splitBlock(CompilationUnit *cUnit,
 
     insn->prev->next = NULL;
     insn->prev = NULL;
+
+    /*
+     * Update the immediate predecessor block pointer so that outgoing edges
+     * can be applied to the proper block.
+     */
+    if (immedPredBlockP) {
+        assert(*immedPredBlockP == origBlock);
+        *immedPredBlockP = bottomBlock;
+    }
     return bottomBlock;
 }
 
 /*
  * Given a code offset, find out the block that starts with it. If the offset
- * is in the middle of an existing block, split it into two.
+ * is in the middle of an existing block, split it into two. If immedPredBlockP
+ * is non-null and is the block being split, update *immedPredBlockP to point
+ * to the bottom block so that outgoing edges can be setup properly (by the
+ * caller).
  */
 static BasicBlock *findBlock(CompilationUnit *cUnit,
                              unsigned int codeOffset,
-                             bool split, bool create)
+                             bool split, bool create,
+                             BasicBlock **immedPredBlockP)
 {
     GrowableList *blockList = &cUnit->blockList;
     BasicBlock *bb;
@@ -621,7 +635,9 @@ static BasicBlock *findBlock(CompilationUnit *cUnit,
         if ((split == true) && (codeOffset > bb->startOffset) &&
             (bb->lastMIRInsn != NULL) &&
             (codeOffset <= bb->lastMIRInsn->offset)) {
-            BasicBlock *newBB = splitBlock(cUnit, codeOffset, bb);
+            BasicBlock *newBB = splitBlock(cUnit, codeOffset, bb,
+                                           bb == *immedPredBlockP ?
+                                               immedPredBlockP : NULL);
             return newBB;
         }
     }
@@ -898,7 +914,9 @@ static void processTryCatchBlocks(CompilationUnit *cUnit)
                       /* split */
                       false,
                       /* create */
-                      true);
+                      true,
+                      /* immedPredBlockP */
+                      NULL);
         }
 
         offset = dexCatchIteratorGetEndOffset(&iterator, pCode);
@@ -942,7 +960,9 @@ static void processCanBranch(CompilationUnit *cUnit, BasicBlock *curBlock,
                                        /* split */
                                        true,
                                        /* create */
-                                       true);
+                                       true,
+                                       /* immedPredBlockP */
+                                       &curBlock);
     curBlock->taken = takenBlock;
     dvmCompilerSetBit(takenBlock->predecessors, curBlock->id);
 
@@ -964,7 +984,9 @@ static void processCanBranch(CompilationUnit *cUnit, BasicBlock *curBlock,
                                                   */
                                                  true,
                                                  /* create */
-                                                 true);
+                                                 true,
+                                                 /* immedPredBlockP */
+                                                 &curBlock);
         curBlock->fallThrough = fallthroughBlock;
         dvmCompilerSetBit(fallthroughBlock->predecessors, curBlock->id);
     } else if (codePtr < codeEnd) {
@@ -974,7 +996,9 @@ static void processCanBranch(CompilationUnit *cUnit, BasicBlock *curBlock,
                       /* split */
                       false,
                       /* create */
-                      true);
+                      true,
+                      /* immedPredBlockP */
+                      NULL);
         }
     }
 }
@@ -1038,7 +1062,9 @@ static void processCanSwitch(CompilationUnit *cUnit, BasicBlock *curBlock,
                                           /* split */
                                           true,
                                           /* create */
-                                          true);
+                                          true,
+                                          /* immedPredBlockP */
+                                          &curBlock);
         SuccessorBlockInfo *successorBlockInfo =
             (SuccessorBlockInfo *) dvmCompilerNew(sizeof(SuccessorBlockInfo),
                                                   false);
@@ -1056,7 +1082,9 @@ static void processCanSwitch(CompilationUnit *cUnit, BasicBlock *curBlock,
                                              /* split */
                                              false,
                                              /* create */
-                                             true);
+                                             true,
+                                             /* immedPredBlockP */
+                                             NULL);
     curBlock->fallThrough = fallthroughBlock;
     dvmCompilerSetBit(fallthroughBlock->predecessors, curBlock->id);
 }
@@ -1099,7 +1127,9 @@ static void processCanThrow(CompilationUnit *cUnit, BasicBlock *curBlock,
                                                /* split */
                                                false,
                                                /* create */
-                                               false);
+                                               false,
+                                               /* immedPredBlockP */
+                                               NULL);
 
             SuccessorBlockInfo *successorBlockInfo =
               (SuccessorBlockInfo *) dvmCompilerNew(sizeof(SuccessorBlockInfo),
@@ -1133,7 +1163,9 @@ static void processCanThrow(CompilationUnit *cUnit, BasicBlock *curBlock,
                                                      /* split */
                                                      false,
                                                      /* create */
-                                                     true);
+                                                     true,
+                                                     /* immedPredBlockP */
+                                                     NULL);
             /*
              * OP_THROW and OP_THROW_VERIFICATION_ERROR are unconditional
              * branches.
@@ -1251,7 +1283,9 @@ bool dvmCompileMethod(const Method *method, JitTranslationInfo *info)
                               /* split */
                               false,
                               /* create */
-                              true);
+                              true,
+                              /* immedPredBlockP */
+                              NULL);
                 }
             }
         } else if (flags & kInstrCanThrow) {
@@ -1265,7 +1299,9 @@ bool dvmCompileMethod(const Method *method, JitTranslationInfo *info)
                                           /* split */
                                           false,
                                           /* create */
-                                          false);
+                                          false,
+                                          /* immedPredBlockP */
+                                          NULL);
         if (nextBlock) {
             /*
              * The next instruction could be the target of a previously parsed
@@ -1405,7 +1441,9 @@ static bool exhaustTrace(CompilationUnit *cUnit, BasicBlock *curBlock)
                                           /* split */
                                           false,
                                           /* create */
-                                          false);
+                                          false,
+                                          /* immedPredBlockP */
+                                          NULL);
         if (nextBlock) {
             /*
              * The next instruction could be the target of a previously parsed

diff --git a/vm/mterp/common/asm-constants.h b/vm/mterp/common/asm-constants.h
index 1313de6..af2275d 100644 (file)
--- a/vm/mterp/common/asm-constants.h
+++ b/vm/mterp/common/asm-constants.h
@@ -193,7 +193,7 @@ MTERP_OFFSET(offThread_methodToCall,      Thread, methodToCall, 164)
 MTERP_OFFSET(offThread_jniLocal_topCookie, \
                                 Thread, jniLocalRefTable.segmentState.all, 168)
 #if defined(WITH_SELF_VERIFICATION)
-MTERP_OFFSET(offThread_shadowSpace,       Thread, shadowSpace, 192)
+MTERP_OFFSET(offThread_shadowSpace,       Thread, shadowSpace, 188)
 #endif
 #else
 MTERP_OFFSET(offThread_jniLocal_topCookie, \