Fix two codegen problems: out-of-bound PC-relative addresses and missing branch to the chaining cell at the end of non-branch-ending basic blocks.

4 files changed, 76 insertions, 126 deletions

diff --git a/vm/compiler/codegen/armv5te/ArchUtility.c b/vm/compiler/codegen/armv5te/ArchUtility.c
index 58b181b7e..a64b54f44 100644
--- a/vm/compiler/codegen/armv5te/ArchUtility.c
+++ b/vm/compiler/codegen/armv5te/ArchUtility.c
@@ -161,11 +161,11 @@ static void dumpLIRInsn(LIR *arg, unsigned char *baseAddr)
     switch(lir->opCode) {
         case ARMV5TE_PSEUDO_TARGET_LABEL:
             break;
-        case ARMV5TE_PSEUDO_CHAINING_CELL_GENERIC:
-            LOGD("-------- chaining cell (generic): 0x%04x\n", dest);
+        case ARMV5TE_PSEUDO_CHAINING_CELL_NORMAL:
+            LOGD("-------- chaining cell (normal): 0x%04x\n", dest);
             break;
-        case ARMV5TE_PSEUDO_CHAINING_CELL_POST_INVOKE:
-            LOGD("-------- chaining cell (post-invoke): 0x%04x\n", dest);
+        case ARMV5TE_PSEUDO_CHAINING_CELL_HOT:
+            LOGD("-------- chaining cell (hot): 0x%04x\n", dest);
             break;
         case ARMV5TE_PSEUDO_CHAINING_CELL_INVOKE:
             LOGD("-------- chaining cell (invoke): %s/%p\n",
diff --git a/vm/compiler/codegen/armv5te/Armv5teLIR.h b/vm/compiler/codegen/armv5te/Armv5teLIR.h
index 208e6c0eb..5268ee8ef 100644
--- a/vm/compiler/codegen/armv5te/Armv5teLIR.h
+++ b/vm/compiler/codegen/armv5te/Armv5teLIR.h
@@ -59,9 +59,9 @@ typedef enum Armv5teConditionCode {
  */
 typedef enum Armv5teOpCode {
     ARMV5TE_PSEUDO_TARGET_LABEL = -10,
-    ARMV5TE_PSEUDO_CHAINING_CELL_POST_INVOKE = -9,
+    ARMV5TE_PSEUDO_CHAINING_CELL_HOT = -9,
     ARMV5TE_PSEUDO_CHAINING_CELL_INVOKE = -8,
-    ARMV5TE_PSEUDO_CHAINING_CELL_GENERIC = -7,
+    ARMV5TE_PSEUDO_CHAINING_CELL_NORMAL = -7,
     ARMV5TE_PSEUDO_DALVIK_BYTECODE_BOUNDARY = -6,
     ARMV5TE_PSEUDO_ALIGN4 = -5,
     ARMV5TE_PSEUDO_PC_RECONSTRUCTION_CELL = -4,
@@ -178,4 +178,6 @@ typedef struct Armv5teLIR {
 #define NEXT_LIR_LVALUE(lir) (lir)->generic.next
 #define PREV_LIR_LVALUE(lir) (lir)->generic.prev
 
+#define CHAIN_CELL_OFFSET_TAG   0xcdab
+
 #endif /* _DALVIK_VM_COMPILER_CODEGEN_ARMV5TE_H */
diff --git a/vm/compiler/codegen/armv5te/Assemble.c b/vm/compiler/codegen/armv5te/Assemble.c
index baa6ced30..12cb15879 100644
--- a/vm/compiler/codegen/armv5te/Assemble.c
+++ b/vm/compiler/codegen/armv5te/Assemble.c
@@ -242,13 +242,12 @@ static bool assembleInstructions(CompilationUnit *cUnit, intptr_t startAddr)
 {
     short *bufferAddr = (short *) cUnit->codeBuffer;
     Armv5teLIR *lir;
-    bool retry = false;
 
     for (lir = (Armv5teLIR *) cUnit->firstLIRInsn; lir; lir = NEXT_LIR(lir)) {
         if (lir->opCode < 0) {
             if ((lir->opCode == ARMV5TE_PSEUDO_ALIGN4) &&
-                (lir->operands[0] == 1) &&
-                !retry) {
+                /* 1 means padding is needed */
+                (lir->operands[0] == 1)) {
                 *bufferAddr++ = PADDING_MOV_R0_R0;
             }
             continue;
@@ -264,6 +263,9 @@ static bool assembleInstructions(CompilationUnit *cUnit, intptr_t startAddr)
                 LOGE("PC-rel distance is not multiples of 4: %d\n", delta);
                 dvmAbort();
             }
+            if (delta > 1023) {
+                return true;
+            }
             lir->operands[1] = delta >> 2;
         } else if (lir->opCode == ARMV5TE_B_COND) {
             Armv5teLIR *targetLIR = (Armv5teLIR *) lir->generic.target;
@@ -271,74 +273,7 @@ static bool assembleInstructions(CompilationUnit *cUnit, intptr_t startAddr)
             intptr_t target = targetLIR->generic.offset;
             int delta = target - pc;
             if (delta > 254 || delta < -256) {
-                /* Pull in the PC reconstruction code inline */
-                if (targetLIR->opCode == ARMV5TE_PSEUDO_PC_RECONSTRUCTION_CELL){
-                    /*
-                     * The original code is:
-                     *
-                     * bxx targetLIR
-                     * origNextLir
-                     *       :
-                     *       :
-                     * targetLIR (a PC reconstruction cell)
-                     *       :
-                     * lastLIR (should be a unconditional branch)
-                     *
-                     * The distance from bxx to targetLIR is too far, so we want
-                     * to rearrange the code to be:
-                     *
-                     * bxx targetLIR
-                     * branchoverLIR to origNextLir
-                     * targetLIR (a PC reconstruction cell)
-                     *       :
-                     * lastLIR (should be a unconditional branch)
-                     * origNextLir
-                     *
-                     * Although doing so adds a unconditional branchover
-                     * instruction, it can be predicted for free by ARM so
-                     * the penalty should be minimal.
-                     */
-                    Armv5teLIR *pcrLIR = targetLIR;
-                    Armv5teLIR *lastLIR = pcrLIR;
-                    Armv5teLIR *origNextLIR = NEXT_LIR(lir);
-
-                    /*
-                     * Find out the last instruction in the PC reconstruction
-                     * cell
-                     */
-                    while (lastLIR->opCode != ARMV5TE_B_UNCOND) {
-                        lastLIR = NEXT_LIR(lastLIR);
-                    }
-
-                    /* Yank out the PCR code */
-                    PREV_LIR_LVALUE(NEXT_LIR(lastLIR)) =
-                        (LIR *) PREV_LIR(targetLIR);
-                    NEXT_LIR_LVALUE(PREV_LIR(targetLIR)) =
-                        (LIR *) NEXT_LIR(lastLIR);
-
-                    /* Create the branch over instruction */
-                    Armv5teLIR *branchoverLIR =
-                        dvmCompilerNew(sizeof(Armv5teLIR), true);
-                    branchoverLIR->opCode = ARMV5TE_B_UNCOND;
-                    branchoverLIR->generic.target = (LIR *) origNextLIR;
-
-                    /* Reconnect the instructions */
-                    NEXT_LIR_LVALUE(lir) = (LIR *) branchoverLIR;
-                    PREV_LIR_LVALUE(branchoverLIR) = (LIR *) lir;
-
-                    NEXT_LIR_LVALUE(branchoverLIR) = (LIR *) targetLIR;
-                    PREV_LIR_LVALUE(targetLIR) = (LIR *) branchoverLIR;
-
-                    NEXT_LIR_LVALUE(lastLIR) = (LIR *) origNextLIR;
-                    PREV_LIR_LVALUE(origNextLIR) = (LIR *) lastLIR;
-
-                    retry = true;
-                    continue;
-                } else {
-                    LOGE("Conditional branch distance out of range: %d\n",
-                         delta);
-                    dvmAbort();
-                }
+                return true;
             }
             lir->operands[0] = delta >> 1;
         } else if (lir->opCode == ARMV5TE_B_UNCOND) {
@@ -368,14 +303,6 @@ static bool assembleInstructions(CompilationUnit *cUnit, intptr_t startAddr)
             NEXT_LIR(lir)->operands[0] = (delta>> 1) & 0x7ff;
         }
 
-        /*
-         * The code offset will be recalculated, just continue to check if
-         * there are other places where code will be rescheduled and do not
-         * write to the output buffer
-         */
-        if (retry) {
-            continue;
-        }
         Armv5teEncodingMap *encoder = &EncodingMap[lir->opCode];
         short bits = encoder->skeleton;
         int i;
@@ -390,7 +317,7 @@ static bool assembleInstructions(CompilationUnit *cUnit, intptr_t startAddr)
         }
         *bufferAddr++ = bits;
     }
-    return retry;
+    return false;
 }
 
 /*
@@ -436,16 +363,13 @@ void dvmCompilerAssembleLIR(CompilationUnit *cUnit)
 {
     LIR *lir;
     Armv5teLIR *armLIR;
-    int offset;
+    int offset = 0;
     int i;
     ChainCellCounts chainCellCounts;
-    u2 chainCellOffset;
     int descSize = jitTraceDescriptionSize(cUnit->traceDesc);
 
-retry:
     /* Beginning offset needs to allow space for chain cell offset */
-    for (armLIR = (Armv5teLIR *) cUnit->firstLIRInsn,
-         offset = CHAIN_CELL_OFFSET_SIZE;
+    for (armLIR = (Armv5teLIR *) cUnit->firstLIRInsn;
          armLIR;
          armLIR = NEXT_LIR(armLIR)) {
         armLIR->generic.offset = offset;
@@ -466,7 +390,15 @@ retry:
     offset = (offset + 3) & ~3;
 
     /* Add space for chain cell counts & trace description */
-    chainCellOffset = offset;
+    u4 chainCellOffset = offset;
+    Armv5teLIR *chainCellOffsetLIR = (Armv5teLIR *) (cUnit->firstLIRInsn);
+    assert(chainCellOffset < 0x10000);
+    assert(chainCellOffsetLIR->opCode == ARMV5TE_16BIT_DATA &&
+           chainCellOffsetLIR->operands[0] == CHAIN_CELL_OFFSET_TAG);
+
+    /* Replace the CHAIN_CELL_OFFSET_TAG with the real value */
+    chainCellOffsetLIR->operands[0] = chainCellOffset;
+
     offset += sizeof(chainCellCounts) + descSize;
 
     assert((offset & 0x3) == 0);  /* Should still be word aligned */
@@ -495,20 +427,24 @@ retry:
         return;
     }
 
-    bool needRetry = assembleInstructions(
+    bool assemblerFailure = assembleInstructions(
         cUnit, (intptr_t) gDvmJit.codeCache + gDvmJit.codeCacheByteUsed);
 
-    if (needRetry)
-        goto retry;
+    /*
+     * Currently the only reason that can cause the assembler to fail is due to
+     * trace length - cut it in half and retry.
+     */
+    if (assemblerFailure) {
+        cUnit->halveInstCount = true;
+        return;
+    }
 
     cUnit->baseAddr = (char *) gDvmJit.codeCache + gDvmJit.codeCacheByteUsed;
+    cUnit->headerSize = CHAIN_CELL_OFFSET_SIZE;
     gDvmJit.codeCacheByteUsed += offset;
 
-    /* Install the chain cell offset */
-    *((char*)cUnit->baseAddr) = chainCellOffset;
-
     /* Install the code block */
-    memcpy((char*)cUnit->baseAddr + 2, cUnit->codeBuffer, chainCellOffset - 2);
+    memcpy((char*)cUnit->baseAddr, cUnit->codeBuffer, chainCellOffset);
     gDvmJit.numCompilations++;
 
     /* Install the chaining cell counts */
@@ -528,9 +464,6 @@ retry:
     /* Flush dcache and invalidate the icache to maintain coherence */
     cacheflush((long)cUnit->baseAddr,
                (long)(cUnit->baseAddr + offset), 0);
-
-    /* Adjust baseAddr to point to executable code */
-    cUnit->baseAddr = (char*)cUnit->baseAddr + CHAIN_CELL_OFFSET_SIZE;
 }
 
 /*
@@ -611,11 +544,11 @@ u4* dvmJitUnchain(void* codeAddr)
         for (j = 0; j < pChainCellCounts->u.count[i]; j++) {
             int targetOffset;
             switch(i) {
-                case CHAINING_CELL_GENERIC:
+                case CHAINING_CELL_NORMAL:
                     targetOffset = offsetof(InterpState,
                           jitToInterpEntries.dvmJitToInterpNormal);
                     break;
-                case CHAINING_CELL_POST_INVOKE:
+                case CHAINING_CELL_HOT:
                 case CHAINING_CELL_INVOKE:
                     targetOffset = offsetof(InterpState,
                           jitToInterpEntries.dvmJitToTraceSelect);
diff --git a/vm/compiler/codegen/armv5te/Codegen.c b/vm/compiler/codegen/armv5te/Codegen.c
index 178e5368b..257272cb4 100644
--- a/vm/compiler/codegen/armv5te/Codegen.c
+++ b/vm/compiler/codegen/armv5te/Codegen.c
@@ -602,10 +602,11 @@ static void genArrayPut(CompilationUnit *cUnit, MIR *mir, Armv5teOpCode inst,
 
     loadValue(cUnit, vArray, r2);
     loadValue(cUnit, vIndex, r3);
-    genNullCheck(cUnit, r2, mir->offset, NULL); /* null object? */
+    /* null object? */
+    Armv5teLIR * pcrLabel = genNullCheck(cUnit, r2, mir->offset, NULL);
     newLIR3(cUnit, ARMV5TE_LDR_RRI5, r0, r2, lenOffset >> 2);  /* Get len */
     newLIR2(cUnit, ARMV5TE_ADD_RI8, r2, dataOffset); /* r2 -> array data */
-    genBoundsCheck(cUnit, r3, r0, mir->offset, NULL);
+    genBoundsCheck(cUnit, r3, r0, mir->offset, pcrLabel);
     /* at this point, r2 points to array, r3 is unscaled index */
     if (scale==3) {
         loadValuePair(cUnit, vSrc, r0, r1);
@@ -2491,9 +2492,9 @@ static bool handleFmt51l(CompilationUnit *cUnit, MIR *mir)
  * Dalvik PC and special-purpose registers are reconstructed here.
  */
 
-/* Chaining cell for normal-ending compiles (eg branches) */
-static void handleGenericChainingCell(CompilationUnit *cUnit,
-                                      unsigned int offset)
+/* Chaining cell for code that may need warmup. */
+static void handleNormalChainingCell(CompilationUnit *cUnit,
+                                     unsigned int offset)
 {
     newLIR3(cUnit, ARMV5TE_LDR_RRI5, r0, rGLUE,
         offsetof(InterpState, jitToInterpEntries.dvmJitToInterpNormal) >> 2);
@@ -2502,11 +2503,11 @@ static void handleGenericChainingCell(CompilationUnit *cUnit,
 }
 
 /*
- * Chaining cell for instructions that immediately following a method
- * invocation.
+ * Chaining cell for instructions that immediately following already translated
+ * code.
  */
-static void handlePostInvokeChainingCell(CompilationUnit *cUnit,
-                                         unsigned int offset)
+static void handleHotChainingCell(CompilationUnit *cUnit,
+                                  unsigned int offset)
 {
     newLIR3(cUnit, ARMV5TE_LDR_RRI5, r0, rGLUE,
         offsetof(InterpState, jitToInterpEntries.dvmJitToTraceSelect) >> 2);
@@ -2559,6 +2560,12 @@ void dvmCompilerMIR2LIR(CompilationUnit *cUnit)
 
     BasicBlock **blockList = cUnit->blockList;
 
+    /*
+     * Reserve space at the beginning of each translation with fillers
+     * + Chain cell count (2 bytes)
+     */
+    newLIR1(cUnit, ARMV5TE_16BIT_DATA, CHAIN_CELL_OFFSET_TAG);
+
     /* Handle the content in each basic block */
     for (i = 0; i < cUnit->numBlocks; i++) {
         blockList[i]->visited = true;
@@ -2578,11 +2585,11 @@ void dvmCompilerMIR2LIR(CompilationUnit *cUnit)
             labelList[i].opCode = ARMV5TE_PSEUDO_NORMAL_BLOCK_LABEL;
         } else {
             switch (blockList[i]->blockType) {
-                case CHAINING_CELL_GENERIC:
-                    labelList[i].opCode = ARMV5TE_PSEUDO_CHAINING_CELL_GENERIC;
+                case CHAINING_CELL_NORMAL:
+                    labelList[i].opCode = ARMV5TE_PSEUDO_CHAINING_CELL_NORMAL;
                     /* handle the codegen later */
                     dvmInsertGrowableList(
-                        &chainingListByType[CHAINING_CELL_GENERIC], (void *) i);
+                        &chainingListByType[CHAINING_CELL_NORMAL], (void *) i);
                     break;
                 case CHAINING_CELL_INVOKE:
                     labelList[i].opCode = ARMV5TE_PSEUDO_CHAINING_CELL_INVOKE;
@@ -2592,12 +2599,12 @@ void dvmCompilerMIR2LIR(CompilationUnit *cUnit)
                     dvmInsertGrowableList(
                         &chainingListByType[CHAINING_CELL_INVOKE], (void *) i);
                     break;
-                case CHAINING_CELL_POST_INVOKE:
+                case CHAINING_CELL_HOT:
                     labelList[i].opCode =
-                        ARMV5TE_PSEUDO_CHAINING_CELL_POST_INVOKE;
+                        ARMV5TE_PSEUDO_CHAINING_CELL_HOT;
                     /* handle the codegen later */
                     dvmInsertGrowableList(
-                        &chainingListByType[CHAINING_CELL_POST_INVOKE],
+                        &chainingListByType[CHAINING_CELL_HOT],
                         (void *) i);
                     break;
                 case PC_RECONSTRUCTION:
@@ -2732,10 +2739,17 @@ void dvmCompilerMIR2LIR(CompilationUnit *cUnit)
                      dalvikFormat);
                 dvmAbort();
                 break;
-            } else {
-              gDvmJit.opHistogram[dalvikOpCode]++;
             }
         }
+        /*
+         * Check if the block is terminated due to trace length constraint -
+         * insert an unconditional branch to the chaining cell.
+         */
+        if (blockList[i]->needFallThroughBranch) {
+            genUnconditionalBranch(cUnit,
+                                   &labelList[blockList[i]->fallThrough->id]);
+        }
+
     }
 
     /* Handle the codegen in predefined order */
@@ -2763,16 +2777,16 @@ void dvmCompilerMIR2LIR(CompilationUnit *cUnit)
 
 
             switch (blockList[blockId]->blockType) {
-                case CHAINING_CELL_GENERIC:
-                    handleGenericChainingCell(cUnit,
+                case CHAINING_CELL_NORMAL:
+                    handleNormalChainingCell(cUnit,
                       blockList[blockId]->startOffset);
                     break;
                 case CHAINING_CELL_INVOKE:
                     handleInvokeChainingCell(cUnit,
                         blockList[blockId]->containingMethod);
                     break;
-                case CHAINING_CELL_POST_INVOKE:
-                    handlePostInvokeChainingCell(cUnit,
+                case CHAINING_CELL_HOT:
+                    handleHotChainingCell(cUnit,
                         blockList[blockId]->startOffset);
                     break;
                 default:
@@ -2797,7 +2811,8 @@ void *dvmCompilerDoWork(CompilerWorkOrder *work)
            res = dvmCompileMethod(work->info);
            break;
        case kWorkOrderTrace:
-           res = dvmCompileTrace(work->info);
+           /* Start compilation with maximally allowed trace length */
+           res = dvmCompileTrace(work->info, JIT_MAX_TRACE_LEN);
            break;
        default:
            res = NULL;