Fix for issue 5884080: Loop formation regression
This CL is the art version of
https://android-git.corp.google.com/g/#/c/159057/1
The basic problem had been partially fixed in the existing art code,
but this CL is a more comprehensive fix.
Change-Id: I59336ed4265a3535d6da7d51c9243430c68536bd
diff --git a/src/compiler/Frontend.cc b/src/compiler/Frontend.cc
index 2eba586..5be51f4 100644
--- a/src/compiler/Frontend.cc
+++ b/src/compiler/Frontend.cc
@@ -115,7 +115,8 @@
/* 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) {
@@ -176,16 +177,28 @@
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) {
+ DCHECK_EQ(*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 not non-null and is the block being split, update *immedPredBlockP to
+ * point to the bottom block so that outgoing edges can be set up 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;
@@ -199,7 +212,9 @@
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;
}
}
@@ -445,7 +460,8 @@
art::CatchHandlerIterator iterator(handlers_ptr);
for (; iterator.HasNext(); iterator.Next()) {
uint32_t address = iterator.GetHandlerAddress();
- findBlock(cUnit, address, false /* split */, true /*create*/);
+ findBlock(cUnit, address, false /* split */, true /*create*/,
+ /* immedPredBlockP */ NULL);
}
handlers_ptr = iterator.EndDataPointer();
}
@@ -484,22 +500,13 @@
LOG(FATAL) << "Unexpected opcode(" << (int)insn->dalvikInsn.opcode
<< ") with kInstrCanBranch set";
}
- /*
- * Some ugliness here. It is possible that findBlock will
- * split the current block. In that case, we need to operate
- * on the 2nd half on the split pair. It isn't directly obvious
- * when this happens, so we infer.
- */
- DCHECK(curBlock->lastMIRInsn == insn);
BasicBlock *takenBlock = findBlock(cUnit, target,
/* split */
true,
/* create */
- true);
- if (curBlock->lastMIRInsn != insn) {
- DCHECK(takenBlock->lastMIRInsn == insn);
- curBlock = curBlock->fallThrough;
- }
+ true,
+ /* immedPredBlockP */
+ &curBlock);
curBlock->taken = takenBlock;
oatSetBit(takenBlock->predecessors, curBlock->id);
@@ -521,7 +528,9 @@
*/
true,
/* create */
- true);
+ true,
+ /* immedPredBlockP */
+ &curBlock);
curBlock->fallThrough = fallthroughBlock;
oatSetBit(fallthroughBlock->predecessors, curBlock->id);
} else if (codePtr < codeEnd) {
@@ -531,7 +540,9 @@
/* split */
false,
/* create */
- true);
+ true,
+ /* immedPredBlockP */
+ NULL);
}
}
return curBlock;
@@ -595,7 +606,9 @@
/* split */
true,
/* create */
- true);
+ true,
+ /* immedPredBlockP */
+ &curBlock);
SuccessorBlockInfo *successorBlockInfo =
(SuccessorBlockInfo *) oatNew(sizeof(SuccessorBlockInfo),
false);
@@ -613,7 +626,9 @@
/* split */
false,
/* create */
- true);
+ true,
+ /* immedPredBlockP */
+ NULL);
curBlock->fallThrough = fallthroughBlock;
oatSetBit(fallthroughBlock->predecessors, curBlock->id);
}
@@ -641,7 +656,8 @@
for (;iterator.HasNext(); iterator.Next()) {
BasicBlock *catchBlock = findBlock(cUnit, iterator.GetHandlerAddress(),
false /* split*/,
- false /* creat */);
+ false /* creat */,
+ NULL /* immedPredBlockP */);
catchBlock->catchEntry = true;
SuccessorBlockInfo *successorBlockInfo =
(SuccessorBlockInfo *) oatNew(sizeof(SuccessorBlockInfo),
@@ -675,7 +691,9 @@
/* split */
false,
/* create */
- true);
+ true,
+ /* immedPredBlockP */
+ NULL);
/*
* OP_THROW is an unconditional branch. NOTE:
* OP_THROW_VERIFICATION_ERROR is also an unconditional
@@ -822,7 +840,9 @@
/* split */
false,
/* create */
- true);
+ true,
+ /* immedPredBlockP */
+ NULL);
}
}
} else if (flags & kInstrCanThrow) {
@@ -836,7 +856,9 @@
/* split */
false,
/* create */
- false);
+ false,
+ /* immedPredBlockP */
+ NULL);
if (nextBlock) {
/*
* The next instruction could be the target of a previously parsed
diff --git a/test/083-jit-regressions/expected.txt b/test/083-jit-regressions/expected.txt
index 1f30d21..4b9ad5b 100644
--- a/test/083-jit-regressions/expected.txt
+++ b/test/083-jit-regressions/expected.txt
@@ -1,3 +1,4 @@
b2296099 passes
b2302318 passes
b2487514 passes
+b5884080 passes
diff --git a/test/083-jit-regressions/info.txt b/test/083-jit-regressions/info.txt
index b791aba..00c24ee 100644
--- a/test/083-jit-regressions/info.txt
+++ b/test/083-jit-regressions/info.txt
@@ -8,3 +8,4 @@
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/test/083-jit-regressions/src/Main.java b/test/083-jit-regressions/src/Main.java
index 1f1dee3..3b596db 100644
--- a/test/083-jit-regressions/src/Main.java
+++ b/test/083-jit-regressions/src/Main.java
@@ -24,6 +24,7 @@
b2296099Test();
b2302318Test();
b2487514Test();
+ b5884080Test();
}
static void b2296099Test() throws Exception {
@@ -105,6 +106,26 @@
" (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 {