From 11ca61259be6ec8e03eaff1e98905232728b3d45 Mon Sep 17 00:00:00 2001 From: Vladimir Marko Date: Thu, 17 Jul 2014 20:50:07 +0100 Subject: [PATCH] Fix GVN to handle normal paths leading to catch entry. When the catch block is empty, the catch entry is actually the normal path block after the try block. Fix the LVN merge for catch entries that didn't expect it during GVN. Bug: 16360024 Change-Id: I9adfc3445245d3fa3c4809d4df1b7b76fbef5ff2 --- compiler/dex/global_value_numbering_test.cc | 24 ++++++++++++++++++++++++ compiler/dex/local_value_numbering.cc | 5 +++++ 2 files changed, 29 insertions(+) diff --git a/compiler/dex/global_value_numbering_test.cc b/compiler/dex/global_value_numbering_test.cc index 40bd9834b..18adbabdf 100644 --- a/compiler/dex/global_value_numbering_test.cc +++ b/compiler/dex/global_value_numbering_test.cc @@ -2090,4 +2090,28 @@ TEST_F(GlobalValueNumberingTestTwoNestedLoops, DISABLED_IFieldAndPhi) { EXPECT_EQ(value_names_[3], value_names_[14]); } +TEST_F(GlobalValueNumberingTest, NormalPathToCatchEntry) { + // When there's an empty catch block, all the exception paths lead to the next block in + // the normal path and we can also have normal "taken" or "fall-through" branches to that + // path. Check that LocalValueNumbering::PruneNonAliasingRefsForCatch() can handle it. + static const BBDef bbs[] = { + DEF_BB(kNullBlock, DEF_SUCC0(), DEF_PRED0()), + DEF_BB(kEntryBlock, DEF_SUCC1(3), DEF_PRED0()), + DEF_BB(kExitBlock, DEF_SUCC0(), DEF_PRED1(4)), + DEF_BB(kDalvikByteCode, DEF_SUCC1(4), DEF_PRED1(1)), + DEF_BB(kDalvikByteCode, DEF_SUCC1(5), DEF_PRED1(3)), + DEF_BB(kDalvikByteCode, DEF_SUCC1(2), DEF_PRED2(3, 4)), + }; + static const MIRDef mirs[] = { + DEF_INVOKE1(4, Instruction::INVOKE_STATIC, 100u), + }; + PrepareBasicBlocks(bbs); + BasicBlock* catch_handler = cu_.mir_graph->GetBasicBlock(5u); + catch_handler->catch_entry = true; + BasicBlock* merge_block = cu_.mir_graph->GetBasicBlock(4u); + std::swap(merge_block->taken, merge_block->fall_through); + PrepareMIRs(mirs); + PerformGVN(); +} + } // namespace art diff --git a/compiler/dex/local_value_numbering.cc b/compiler/dex/local_value_numbering.cc index d5fd6feec..ef893fe0b 100644 --- a/compiler/dex/local_value_numbering.cc +++ b/compiler/dex/local_value_numbering.cc @@ -445,6 +445,11 @@ void LocalValueNumbering::MergeMemoryVersions(bool clobbered_catch) { void LocalValueNumbering::PruneNonAliasingRefsForCatch() { for (const LocalValueNumbering* lvn : gvn_->merge_lvns_) { const BasicBlock* bb = gvn_->GetBasicBlock(lvn->Id()); + if (UNLIKELY(bb->taken == id_) || UNLIKELY(bb->fall_through == id_)) { + // Non-exceptional path to a catch handler means that the catch block was actually + // empty and all exceptional paths lead to the shared path after that empty block. + continue; + } DCHECK_EQ(bb->taken, kNullBlock); DCHECK_NE(bb->fall_through, kNullBlock); const BasicBlock* fall_through_bb = gvn_->GetBasicBlock(bb->fall_through); -- 2.11.0