Revert "Revert "ART: Ignore try blocks with no throwing instructions""
The original CL broke libcore tests because monitor-exit instructions
did not have any side-effects and got removed by DCE once not labelled
throwing any more.
This reverts commit efe374d7c25c1d48945a9198d96469de99e0c1bd.
Change-Id: I624c0f91676d9baaada6f33be9d7091f68d57535
diff --git a/compiler/optimizing/builder.cc b/compiler/optimizing/builder.cc
index 8551382..8bf744d 100644
--- a/compiler/optimizing/builder.cc
+++ b/compiler/optimizing/builder.cc
@@ -259,14 +259,24 @@
return false;
}
-bool HGraphBuilder::IsBlockInPcRange(HBasicBlock* block,
- uint32_t dex_pc_start,
- uint32_t dex_pc_end) {
- uint32_t dex_pc = block->GetDexPc();
- return block != entry_block_
- && block != exit_block_
- && dex_pc >= dex_pc_start
- && dex_pc < dex_pc_end;
+static const DexFile::TryItem* GetTryItem(HBasicBlock* block,
+ const DexFile::CodeItem& code_item,
+ const ArenaBitVector& can_block_throw) {
+ DCHECK(!block->IsSingleTryBoundary());
+
+ // Block does not contain throwing instructions. Even if it is covered by
+ // a TryItem, we will consider it not in a try block.
+ if (!can_block_throw.IsBitSet(block->GetBlockId())) {
+ return nullptr;
+ }
+
+ // Instructions in the block may throw. Find a TryItem covering this block.
+ int32_t try_item_idx = DexFile::FindTryItem(code_item, block->GetDexPc());
+ if (try_item_idx == -1) {
+ return nullptr;
+ } else {
+ return DexFile::GetTryItems(code_item, try_item_idx);
+ }
}
void HGraphBuilder::CreateBlocksForTryCatch(const DexFile::CodeItem& code_item) {
@@ -327,30 +337,41 @@
return;
}
+ const size_t num_blocks = graph_->GetBlocks().Size();
+ ArenaBitVector can_block_throw(arena_, num_blocks, false);
+
+ // Scan blocks and mark those which contain throwing instructions.
+ for (size_t block_id = 0; block_id < num_blocks; ++block_id) {
+ HBasicBlock* block = graph_->GetBlocks().Get(block_id);
+ for (HInstructionIterator insn(block->GetInstructions()); !insn.Done(); insn.Advance()) {
+ if (insn.Current()->CanThrow()) {
+ can_block_throw.SetBit(block_id);
+ break;
+ }
+ }
+ }
+
// Iterate over all blocks, find those covered by some TryItem and:
// (a) split edges which enter/exit the try range,
// (b) create TryBoundary instructions in the new blocks,
// (c) link the new blocks to corresponding exception handlers.
// We cannot iterate only over blocks in `branch_targets_` because switch-case
// blocks share the same dex_pc.
- for (size_t block_id = 1, e = graph_->GetBlocks().Size(); block_id < e; ++block_id) {
+ for (size_t block_id = 1; block_id < num_blocks - 1; ++block_id) {
HBasicBlock* try_block = graph_->GetBlocks().Get(block_id);
// Iteration starts from 1 to skip the entry block.
DCHECK_NE(try_block, entry_block_);
- // Exit block has not yet been added to the graph at this point.
+ // Iteration ends at num_blocks - 1 to skip the exit block.
DCHECK_NE(try_block, exit_block_);
// TryBoundary blocks are added at the end of the list and not iterated over.
DCHECK(!try_block->IsSingleTryBoundary());
// Find the TryItem for this block.
- int32_t try_item_idx = DexFile::FindTryItem(code_item, try_block->GetDexPc());
- if (try_item_idx == -1) {
+ const DexFile::TryItem* try_item = GetTryItem(try_block, code_item, can_block_throw);
+ if (try_item == nullptr) {
continue;
}
- const DexFile::TryItem& try_item = *DexFile::GetTryItems(code_item, try_item_idx);
- uint32_t try_start = try_item.start_addr_;
- uint32_t try_end = try_start + try_item.insn_count_;
if (try_block->IsCatchBlock()) {
// Catch blocks are always considered an entry point into the TryItem in
@@ -373,7 +394,7 @@
DCHECK(split_position != nullptr);
HBasicBlock* catch_block = try_block;
try_block = catch_block->SplitBefore(split_position);
- SplitTryBoundaryEdge(catch_block, try_block, HTryBoundary::kEntry, code_item, try_item);
+ SplitTryBoundaryEdge(catch_block, try_block, HTryBoundary::kEntry, code_item, *try_item);
} else {
// For non-catch blocks, find predecessors which are not covered by the
// same TryItem range. Such edges enter the try block and will have
@@ -385,12 +406,14 @@
// TryItem. We split it again and insert an entry point.
if (kIsDebugBuild) {
HTryBoundary* last_insn = predecessor->GetLastInstruction()->AsTryBoundary();
+ const DexFile::TryItem* predecessor_try_item =
+ GetTryItem(predecessor->GetSinglePredecessor(), code_item, can_block_throw);
DCHECK(!last_insn->IsEntry());
DCHECK_EQ(last_insn->GetNormalFlowSuccessor(), try_block);
DCHECK(try_block->IsFirstIndexOfPredecessor(predecessor, i));
- DCHECK(!IsBlockInPcRange(predecessor->GetSinglePredecessor(), try_start, try_end));
+ DCHECK_NE(try_item, predecessor_try_item);
}
- } else if (!IsBlockInPcRange(predecessor, try_start, try_end)) {
+ } else if (GetTryItem(predecessor, code_item, can_block_throw) != try_item) {
// This is an entry point into the TryItem and the edge has not been
// split yet. That means that `predecessor` is not in a TryItem, or
// it is in a different TryItem and we happened to iterate over this
@@ -399,7 +422,7 @@
// Not an edge on the boundary of the try block.
continue;
}
- SplitTryBoundaryEdge(predecessor, try_block, HTryBoundary::kEntry, code_item, try_item);
+ SplitTryBoundaryEdge(predecessor, try_block, HTryBoundary::kEntry, code_item, *try_item);
}
}
@@ -416,11 +439,13 @@
// TryItem. We split it again and insert an exit.
if (kIsDebugBuild) {
HTryBoundary* last_insn = successor->GetLastInstruction()->AsTryBoundary();
+ const DexFile::TryItem* successor_try_item =
+ GetTryItem(last_insn->GetNormalFlowSuccessor(), code_item, can_block_throw);
DCHECK_EQ(try_block, successor->GetSinglePredecessor());
DCHECK(last_insn->IsEntry());
- DCHECK(!IsBlockInPcRange(last_insn->GetNormalFlowSuccessor(), try_start, try_end));
+ DCHECK_NE(try_item, successor_try_item);
}
- } else if (!IsBlockInPcRange(successor, try_start, try_end)) {
+ } else if (GetTryItem(successor, code_item, can_block_throw) != try_item) {
// This is an exit out of the TryItem and the edge has not been split
// yet. That means that either `successor` is not in a TryItem, or it
// is in a different TryItem and we happened to iterate over this
@@ -437,7 +462,7 @@
// Not an edge on the boundary of the try block.
continue;
}
- SplitTryBoundaryEdge(try_block, successor, HTryBoundary::kExit, code_item, try_item);
+ SplitTryBoundaryEdge(try_block, successor, HTryBoundary::kExit, code_item, *try_item);
}
}
}
@@ -496,14 +521,14 @@
// Add the suspend check to the entry block.
entry_block_->AddInstruction(new (arena_) HSuspendCheck(0));
entry_block_->AddInstruction(new (arena_) HGoto());
+ // Add the exit block at the end.
+ graph_->AddBlock(exit_block_);
// Iterate over blocks covered by TryItems and insert TryBoundaries at entry
// and exit points. This requires all control-flow instructions and
// non-exceptional edges to have been created.
InsertTryBoundaryBlocks(code_item);
- // Add the exit block at the end to give it the highest id.
- graph_->AddBlock(exit_block_);
return true;
}
diff --git a/compiler/optimizing/builder.h b/compiler/optimizing/builder.h
index e487255..7098eb8 100644
--- a/compiler/optimizing/builder.h
+++ b/compiler/optimizing/builder.h
@@ -98,9 +98,6 @@
HBasicBlock* FindBlockStartingAt(int32_t dex_pc) const;
HBasicBlock* FindOrCreateBlockStartingAt(int32_t dex_pc);
- // Returns whether the dex_pc of `block` lies within the given range.
- bool IsBlockInPcRange(HBasicBlock* block, uint32_t dex_pc_start, uint32_t dex_pc_end);
-
// Adds new blocks to `branch_targets_` starting at the limits of TryItems and
// their exception handlers.
void CreateBlocksForTryCatch(const DexFile::CodeItem& code_item);
diff --git a/compiler/optimizing/graph_visualizer.cc b/compiler/optimizing/graph_visualizer.cc
index 504c141..37c060c 100644
--- a/compiler/optimizing/graph_visualizer.cc
+++ b/compiler/optimizing/graph_visualizer.cc
@@ -357,6 +357,10 @@
StartAttributeStream("kind") << barrier->GetBarrierKind();
}
+ void VisitMonitorOperation(HMonitorOperation* monitor) OVERRIDE {
+ StartAttributeStream("kind") << (monitor->IsEnter() ? "enter" : "exit");
+ }
+
void VisitLoadClass(HLoadClass* load_class) OVERRIDE {
StartAttributeStream("gen_clinit_check") << std::boolalpha
<< load_class->MustGenerateClinitCheck() << std::noboolalpha;
diff --git a/compiler/optimizing/nodes.h b/compiler/optimizing/nodes.h
index e4a7aa6..59255d1 100644
--- a/compiler/optimizing/nodes.h
+++ b/compiler/optimizing/nodes.h
@@ -4131,13 +4131,19 @@
};
HMonitorOperation(HInstruction* object, OperationKind kind, uint32_t dex_pc)
- : HTemplateInstruction(SideEffects::None()), kind_(kind), dex_pc_(dex_pc) {
+ : HTemplateInstruction(SideEffects::ChangesSomething()), kind_(kind), dex_pc_(dex_pc) {
SetRawInputAt(0, object);
}
// Instruction may throw a Java exception, so we need an environment.
- bool NeedsEnvironment() const OVERRIDE { return true; }
- bool CanThrow() const OVERRIDE { return true; }
+ bool NeedsEnvironment() const OVERRIDE { return CanThrow(); }
+
+ bool CanThrow() const OVERRIDE {
+ // Verifier guarantees that monitor-exit cannot throw.
+ // This is important because it allows the HGraphBuilder to remove
+ // a dead throw-catch loop generated for `synchronized` blocks/methods.
+ return IsEnter();
+ }
uint32_t GetDexPc() const OVERRIDE { return dex_pc_; }
diff --git a/test/510-checker-try-catch/smali/Builder.smali b/test/510-checker-try-catch/smali/Builder.smali
index 4ea7b61..87e4064 100644
--- a/test/510-checker-try-catch/smali/Builder.smali
+++ b/test/510-checker-try-catch/smali/Builder.smali
@@ -713,20 +713,20 @@
## CHECK-START: int Builder.testSwitchTryExit(int, int, int, int) builder (after)
## CHECK: name "B0"
-## CHECK: successors "<<BEnterTry:B\d+>>"
+## CHECK: successors "<<BEnterTry1:B\d+>>"
## CHECK: name "<<BPSwitch0:B\d+>>"
-## CHECK: predecessors "<<BEnterTry>>"
-## CHECK: successors "<<BTry2:B\d+>>" "<<BPSwitch1:B\d+>>"
+## CHECK: predecessors "<<BEnterTry1>>"
+## CHECK: successors "<<BTry2:B\d+>>" "<<BExitTry1:B\d+>>"
## CHECK: If
-## CHECK: name "<<BPSwitch1>>"
-## CHECK: predecessors "<<BPSwitch0>>"
-## CHECK: successors "<<BExitTry1:B\d+>>" "<<BTry1:B\d+>>"
+## CHECK: name "<<BPSwitch1:B\d+>>"
+## CHECK: predecessors "<<BExitTry1>>"
+## CHECK: successors "<<BOutside:B\d+>>" "<<BEnterTry2:B\d+>>"
## CHECK: If
-## CHECK: name "<<BTry1>>"
-## CHECK: predecessors "<<BPSwitch1>>"
+## CHECK: name "<<BTry1:B\d+>>"
+## CHECK: predecessors "<<BEnterTry2>>"
## CHECK: successors "<<BTry2>>"
## CHECK: Div
@@ -735,28 +735,34 @@
## CHECK: successors "<<BExitTry2:B\d+>>"
## CHECK: Div
-## CHECK: name "<<BOutside:B\d+>>"
-## CHECK: predecessors "<<BExitTry1>>" "<<BExitTry2>>"
+## CHECK: name "<<BOutside>>"
+## CHECK: predecessors "<<BPSwitch1>>" "<<BExitTry2>>"
## CHECK: successors "<<BCatchReturn:B\d+>>"
## CHECK: Div
## CHECK: name "<<BCatchReturn>>"
-## CHECK: predecessors "<<BOutside>>" "<<BEnterTry>>" "<<BExitTry1>>" "<<BExitTry2>>"
+## CHECK: predecessors "<<BOutside>>" "<<BEnterTry1>>" "<<BExitTry1>>" "<<BEnterTry2>>" "<<BExitTry2>>"
## CHECK: flags "catch_block"
## CHECK: Return
-## CHECK: name "<<BEnterTry>>"
+## CHECK: name "<<BEnterTry1>>"
## CHECK: predecessors "B0"
## CHECK: successors "<<BPSwitch0>>"
## CHECK: xhandlers "<<BCatchReturn>>"
## CHECK: TryBoundary kind:entry
## CHECK: name "<<BExitTry1>>"
-## CHECK: predecessors "<<BPSwitch1>>"
-## CHECK: successors "<<BOutside>>"
+## CHECK: predecessors "<<BPSwitch0>>"
+## CHECK: successors "<<BPSwitch1>>"
## CHECK: xhandlers "<<BCatchReturn>>"
## CHECK: TryBoundary kind:exit
+## CHECK: name "<<BEnterTry2>>"
+## CHECK: predecessors "<<BPSwitch1>>"
+## CHECK: successors "<<BTry1>>"
+## CHECK: xhandlers "<<BCatchReturn>>"
+## CHECK: TryBoundary kind:entry
+
## CHECK: name "<<BExitTry2>>"
## CHECK: predecessors "<<BTry2>>"
## CHECK: successors "<<BOutside>>"
@@ -767,6 +773,7 @@
.registers 4
:try_start
+ div-int/2addr p0, p1
packed-switch p0, :pswitch_data
div-int/2addr p0, p1
@@ -809,6 +816,10 @@
## CHECK: flags "catch_block"
## CHECK: StoreLocal [v0,<<Minus1>>]
+## CHECK: name "<<BExit>>"
+## CHECK: predecessors "<<BExitTry>>" "<<BCatch>>"
+## CHECK: Exit
+
## CHECK: name "<<BEnterTry>>"
## CHECK: predecessors "B0"
## CHECK: successors "<<BTry>>"
@@ -821,10 +832,6 @@
## CHECK: xhandlers "<<BCatch>>"
## CHECK: TryBoundary kind:exit
-## CHECK: name "<<BExit>>"
-## CHECK: predecessors "<<BExitTry>>" "<<BCatch>>"
-## CHECK: Exit
-
.method public static testThrow(Ljava/lang/Exception;)I
.registers 2
@@ -852,6 +859,9 @@
## CHECK: name "<<BReturn:B\d+>>"
## CHECK: predecessors "<<BExitTry>>"
+## CHECK: successors "<<BExit:B\d+>>"
+
+## CHECK: name "<<BExit>>"
## CHECK: name "<<BTry:B\d+>>"
## CHECK: predecessors "<<BEnterTry>>"
@@ -956,48 +966,51 @@
## CHECK: successors "<<BCatch1:B\d+>>"
## CHECK: name "<<BCatch1>>"
-## CHECK: predecessors "B0" "<<BEnter2:B\d+>>" "<<BExit2:B\d+>>"
-## CHECK: successors "<<BEnter1:B\d+>>"
+## CHECK: predecessors "B0" "<<BEnterTry2:B\d+>>" "<<BExitTry2:B\d+>>"
+## CHECK: successors "<<BEnterTry1:B\d+>>"
## CHECK: flags "catch_block"
## CHECK: name "<<BCatch2:B\d+>>"
-## CHECK: predecessors "<<BExit1:B\d+>>" "<<BEnter1>>" "<<BExit1>>"
-## CHECK: successors "<<BEnter2>>"
+## CHECK: predecessors "<<BExitTry1:B\d+>>" "<<BEnterTry1>>" "<<BExitTry1>>"
+## CHECK: successors "<<BEnterTry2>>"
## CHECK: flags "catch_block"
## CHECK: name "<<BReturn:B\d+>>"
-## CHECK: predecessors "<<BExit2>>"
+## CHECK: predecessors "<<BExitTry2>>"
+## CHECK: successors "<<BExit:B\d+>>"
## CHECK: Return
+## CHECK: name "<<BExit>>"
+
## CHECK: name "<<BTry1:B\d+>>"
-## CHECK: predecessors "<<BEnter1>>"
-## CHECK: successors "<<BExit1>>"
+## CHECK: predecessors "<<BEnterTry1>>"
+## CHECK: successors "<<BExitTry1>>"
## CHECK: Div
-## CHECK: name "<<BEnter1>>"
+## CHECK: name "<<BEnterTry1>>"
## CHECK: predecessors "<<BCatch1>>"
## CHECK: successors "<<BTry1>>"
## CHECK: xhandlers "<<BCatch2>>"
## CHECK: TryBoundary kind:entry
-## CHECK: name "<<BExit1>>"
+## CHECK: name "<<BExitTry1>>"
## CHECK: predecessors "<<BTry1>>"
## CHECK: successors "<<BCatch2>>"
## CHECK: xhandlers "<<BCatch2>>"
## CHECK: TryBoundary kind:exit
## CHECK: name "<<BTry2:B\d+>>"
-## CHECK: predecessors "<<BEnter2>>"
-## CHECK: successors "<<BExit2>>"
+## CHECK: predecessors "<<BEnterTry2>>"
+## CHECK: successors "<<BExitTry2>>"
## CHECK: Div
-## CHECK: name "<<BEnter2>>"
+## CHECK: name "<<BEnterTry2>>"
## CHECK: predecessors "<<BCatch2>>"
## CHECK: successors "<<BTry2>>"
## CHECK: xhandlers "<<BCatch1>>"
## CHECK: TryBoundary kind:entry
-## CHECK: name "<<BExit2>>"
+## CHECK: name "<<BExitTry2>>"
## CHECK: predecessors "<<BTry2>>"
## CHECK: successors "<<BReturn>>"
## CHECK: xhandlers "<<BCatch1>>"
@@ -1131,3 +1144,29 @@
:try_end
.catchall {:try_start .. :try_end} :catch_all
.end method
+
+## CHECK-START: int Builder.testSynchronized(java.lang.Object) builder (after)
+## CHECK: flags "catch_block"
+## CHECK-NOT: end_block
+## CHECK: MonitorOperation kind:exit
+
+.method public static testSynchronized(Ljava/lang/Object;)I
+ .registers 2
+
+ monitor-enter p0
+
+ :try_start_9
+ invoke-virtual {p0}, Ljava/lang/Object;->hashCode()I
+ move-result v0
+
+ monitor-exit p0
+ return v0
+
+ :catchall_11
+ move-exception v0
+ monitor-exit p0
+ :try_end_15
+ .catchall {:try_start_9 .. :try_end_15} :catchall_11
+
+ throw v0
+.end method
