Ensure we can always enter OSR code
When the the loop header is not the target of a back-edge,
we used to never enter the OSR code even if it's been compiled.
Test: testrunner.py --host --jit -t 570-checker-osr-locals
(it used to get stuck, you can kill the dalvikvm to check that
the weirdLoop was OSR-compiled)
Bug: 136743846
Change-Id: Iae55463eff92adccf9adec842e04f8ff6d9d8568
diff --git a/compiler/optimizing/block_builder.cc b/compiler/optimizing/block_builder.cc
index a5f78ca..e1f061a 100644
--- a/compiler/optimizing/block_builder.cc
+++ b/compiler/optimizing/block_builder.cc
@@ -398,6 +398,48 @@
}
}
+void HBasicBlockBuilder::InsertSynthesizedLoopsForOsr() {
+ ArenaSet<uint32_t> targets(allocator_->Adapter(kArenaAllocGraphBuilder));
+ // Collect basic blocks that are targets of a negative branch.
+ for (const DexInstructionPcPair& pair : code_item_accessor_) {
+ const uint32_t dex_pc = pair.DexPc();
+ const Instruction& instruction = pair.Inst();
+ if (instruction.IsBranch()) {
+ uint32_t target_dex_pc = dex_pc + instruction.GetTargetOffset();
+ if (target_dex_pc < dex_pc) {
+ HBasicBlock* block = GetBlockAt(target_dex_pc);
+ CHECK_NE(kNoDexPc, block->GetDexPc());
+ targets.insert(block->GetBlockId());
+ }
+ } else if (instruction.IsSwitch()) {
+ DexSwitchTable table(instruction, dex_pc);
+ for (DexSwitchTableIterator s_it(table); !s_it.Done(); s_it.Advance()) {
+ uint32_t target_dex_pc = dex_pc + s_it.CurrentTargetOffset();
+ if (target_dex_pc < dex_pc) {
+ HBasicBlock* block = GetBlockAt(target_dex_pc);
+ CHECK_NE(kNoDexPc, block->GetDexPc());
+ targets.insert(block->GetBlockId());
+ }
+ }
+ }
+ }
+
+ // Insert synthesized loops before the collected blocks.
+ for (uint32_t block_id : targets) {
+ HBasicBlock* block = graph_->GetBlocks()[block_id];
+ HBasicBlock* loop_block = new (allocator_) HBasicBlock(graph_, block->GetDexPc());
+ graph_->AddBlock(loop_block);
+ while (!block->GetPredecessors().empty()) {
+ block->GetPredecessors()[0]->ReplaceSuccessor(block, loop_block);
+ }
+ loop_block->AddSuccessor(loop_block);
+ loop_block->AddSuccessor(block);
+ // We loop on false - we know this won't be optimized later on as the loop
+ // is marked irreducible, which disables loop optimizations.
+ loop_block->AddInstruction(new (allocator_) HIf(graph_->GetIntConstant(0), kNoDexPc));
+ }
+}
+
bool HBasicBlockBuilder::Build() {
DCHECK(code_item_accessor_.HasCodeItem());
DCHECK(graph_->GetBlocks().empty());
@@ -413,6 +455,10 @@
ConnectBasicBlocks();
InsertTryBoundaryBlocks();
+ if (graph_->IsCompilingOsr()) {
+ InsertSynthesizedLoopsForOsr();
+ }
+
return true;
}
diff --git a/compiler/optimizing/block_builder.h b/compiler/optimizing/block_builder.h
index 2c1f034..42a3f32 100644
--- a/compiler/optimizing/block_builder.h
+++ b/compiler/optimizing/block_builder.h
@@ -59,6 +59,11 @@
void ConnectBasicBlocks();
void InsertTryBoundaryBlocks();
+ // To ensure branches with negative offsets can always OSR jump to compiled
+ // code, we insert synthesized loops before each block that is the target of a
+ // negative branch.
+ void InsertSynthesizedLoopsForOsr();
+
// Helper method which decides whether `catch_block` may have live normal
// predecessors and thus whether a synthetic catch block needs to be created
// to avoid mixing normal and exceptional predecessors.
diff --git a/test/570-checker-osr-locals/smali/WeirdLoop.smali b/test/570-checker-osr-locals/smali/WeirdLoop.smali
new file mode 100644
index 0000000..13cb4f9
--- /dev/null
+++ b/test/570-checker-osr-locals/smali/WeirdLoop.smali
@@ -0,0 +1,39 @@
+# Copyright (C) 2019 The Android Open Source Project
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+.class public LWeirdLoop;
+
+.super Ljava/lang/Object;
+
+.method public static weirdLoop()I
+ .registers 3
+ invoke-static {}, LMain;->$noinline$magicValue()I
+ move-result v0
+ const-string v1, "weirdLoop"
+ invoke-static {v1}, LMain;->isInInterpreter(Ljava/lang/String;)Z
+ move-result v2
+ if-eqz v2, :end
+ goto :mid
+
+ :top
+ invoke-static {}, LMain;->$noinline$magicValue()I
+ move-result v0
+ :mid
+ invoke-static {v1}, LMain;->isInOsrCode(Ljava/lang/String;)Z
+ move-result v2
+ if-eqz v2, :top
+ :end
+ return v0
+.end method
+
diff --git a/test/570-checker-osr-locals/src/Main.java b/test/570-checker-osr-locals/src/Main.java
index f4b3ab6..c215fa0 100644
--- a/test/570-checker-osr-locals/src/Main.java
+++ b/test/570-checker-osr-locals/src/Main.java
@@ -19,6 +19,7 @@
System.loadLibrary(args[0]);
while (runTests(true));
runTests(false);
+ runSmaliTest();
}
public static boolean runTests(boolean warmup) {
@@ -65,6 +66,18 @@
return true;
}
+ public static void runSmaliTest() {
+ try {
+ Class<?> c = Class.forName("WeirdLoop");
+ int result = (int) c.getDeclaredMethod("weirdLoop").invoke(null);
+ if (result != 42) {
+ throw new Error("Unexpected result: " + result);
+ }
+ } catch (Throwable t) {
+ t.printStackTrace();
+ }
+ }
+
public static int $noinline$magicValue() {
return 42;
}