From fb552d7061746f7a90fdd5002696e255e2e15c35 Mon Sep 17 00:00:00 2001 From: David Brazdil Date: Mon, 2 Nov 2015 20:24:24 +0000 Subject: [PATCH] Revert "ART: Update DCE to work with try/catch" This reverts commit ce52901e2c8377fc1c331ae0faf7fbcb46b9da97. Change-Id: I6b3a1f2a3dc036030b089b0df2005ecefa66b949 --- compiler/optimizing/dead_code_elimination.cc | 12 +- compiler/optimizing/nodes.cc | 39 +--- test/543-checker-dce-trycatch/expected.txt | 0 test/543-checker-dce-trycatch/info.txt | 1 - test/543-checker-dce-trycatch/smali/TestCase.smali | 200 --------------------- test/543-checker-dce-trycatch/src/Main.java | 66 ------- 6 files changed, 15 insertions(+), 303 deletions(-) delete mode 100644 test/543-checker-dce-trycatch/expected.txt delete mode 100644 test/543-checker-dce-trycatch/info.txt delete mode 100644 test/543-checker-dce-trycatch/smali/TestCase.smali delete mode 100644 test/543-checker-dce-trycatch/src/Main.java diff --git a/compiler/optimizing/dead_code_elimination.cc b/compiler/optimizing/dead_code_elimination.cc index 02e5dab3d..9754043f3 100644 --- a/compiler/optimizing/dead_code_elimination.cc +++ b/compiler/optimizing/dead_code_elimination.cc @@ -123,21 +123,20 @@ void HDeadCodeElimination::RemoveDeadBlocks() { } // If we removed at least one block, we need to recompute the full - // dominator tree and try block membership. + // dominator tree. if (removed_one_or_more_blocks) { graph_->ClearDominanceInformation(); graph_->ComputeDominanceInformation(); - graph_->ComputeTryBlockInformation(); } // Connect successive blocks created by dead branches. Order does not matter. for (HReversePostOrderIterator it(*graph_); !it.Done();) { HBasicBlock* block = it.Current(); - if (block->IsEntryBlock() || !block->GetLastInstruction()->IsGoto()) { + if (block->IsEntryBlock() || block->GetSuccessors().size() != 1u) { it.Advance(); continue; } - HBasicBlock* successor = block->GetSingleSuccessor(); + HBasicBlock* successor = block->GetSuccessors()[0]; if (successor->IsExitBlock() || successor->GetPredecessors().size() != 1u) { it.Advance(); continue; @@ -177,7 +176,10 @@ void HDeadCodeElimination::RemoveDeadInstructions() { } void HDeadCodeElimination::Run() { - RemoveDeadBlocks(); + if (!graph_->HasTryCatch()) { + // TODO: Update dead block elimination and enable for try/catch. + RemoveDeadBlocks(); + } SsaRedundantPhiElimination(graph_).Run(); RemoveDeadInstructions(); } diff --git a/compiler/optimizing/nodes.cc b/compiler/optimizing/nodes.cc index 4d79b5577..68fb0acf7 100644 --- a/compiler/optimizing/nodes.cc +++ b/compiler/optimizing/nodes.cc @@ -366,11 +366,7 @@ void HGraph::ComputeTryBlockInformation() { HBasicBlock* first_predecessor = block->GetPredecessors()[0]; DCHECK(!block->IsLoopHeader() || !block->GetLoopInformation()->IsBackEdge(*first_predecessor)); const HTryBoundary* try_entry = first_predecessor->ComputeTryEntryOfSuccessors(); - if (try_entry != nullptr && - (block->GetTryCatchInformation() == nullptr || - try_entry != &block->GetTryCatchInformation()->GetTryEntry())) { - // We are either setting try block membership for the first time or it - // has changed. + if (try_entry != nullptr) { block->SetTryCatchInformation(new (arena_) TryCatchInformation(*try_entry)); } } @@ -1376,30 +1372,13 @@ void HBasicBlock::DisconnectAndDelete() { // instructions. for (HBasicBlock* predecessor : predecessors_) { HInstruction* last_instruction = predecessor->GetLastInstruction(); - if (last_instruction->IsTryBoundary() && !IsCatchBlock()) { - // This block is the only normal-flow successor of the TryBoundary which - // makes `predecessor` dead. Since DCE removes blocks in post order, - // exception handlers of this TryBoundary were already visited and any - // remaining handlers therefore must be live. We remove `predecessor` from - // their list of predecessors. - DCHECK_EQ(last_instruction->AsTryBoundary()->GetNormalFlowSuccessor(), this); - while (predecessor->GetSuccessors().size() > 1) { - HBasicBlock* handler = predecessor->GetSuccessors()[1]; - DCHECK(handler->IsCatchBlock()); - predecessor->RemoveSuccessor(handler); - handler->RemovePredecessor(predecessor); - } - } - predecessor->RemoveSuccessor(this); uint32_t num_pred_successors = predecessor->GetSuccessors().size(); if (num_pred_successors == 1u) { // If we have one successor after removing one, then we must have - // had an HIf, HPackedSwitch or HTryBoundary, as they have more than one - // successor. Replace those with a HGoto. - DCHECK(last_instruction->IsIf() || - last_instruction->IsPackedSwitch() || - (last_instruction->IsTryBoundary() && IsCatchBlock())); + // had an HIf or HPackedSwitch, as they have more than one successor. + // Replace those with a HGoto. + DCHECK(last_instruction->IsIf() || last_instruction->IsPackedSwitch()); predecessor->RemoveInstruction(last_instruction); predecessor->AddInstruction(new (graph_->GetArena()) HGoto(last_instruction->GetDexPc())); } else if (num_pred_successors == 0u) { @@ -1408,12 +1387,10 @@ void HBasicBlock::DisconnectAndDelete() { // SSAChecker fails unless it is not removed during the pass too. predecessor->RemoveInstruction(last_instruction); } else { - // There are multiple successors left. The removed block might be a successor - // of a PackedSwitch which will be completely removed (perhaps replaced with - // a Goto), or we are deleting a catch block from a TryBoundary. In either - // case, leave `last_instruction` as is for now. - DCHECK(last_instruction->IsPackedSwitch() || - (last_instruction->IsTryBoundary() && IsCatchBlock())); + // There are multiple successors left. This must come from a HPackedSwitch + // and we are in the middle of removing the HPackedSwitch. Like above, leave + // this alone, and the SSAChecker will fail if it is not removed as well. + DCHECK(last_instruction->IsPackedSwitch()); } } predecessors_.clear(); diff --git a/test/543-checker-dce-trycatch/expected.txt b/test/543-checker-dce-trycatch/expected.txt deleted file mode 100644 index e69de29bb..000000000 diff --git a/test/543-checker-dce-trycatch/info.txt b/test/543-checker-dce-trycatch/info.txt deleted file mode 100644 index e541938f6..000000000 --- a/test/543-checker-dce-trycatch/info.txt +++ /dev/null @@ -1 +0,0 @@ -Tests removal of try/catch blocks by DCE. \ No newline at end of file diff --git a/test/543-checker-dce-trycatch/smali/TestCase.smali b/test/543-checker-dce-trycatch/smali/TestCase.smali deleted file mode 100644 index 3e7193723..000000000 --- a/test/543-checker-dce-trycatch/smali/TestCase.smali +++ /dev/null @@ -1,200 +0,0 @@ -# Copyright (C) 2015 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 LTestCase; -.super Ljava/lang/Object; - -.method private static $inline$False()Z - .registers 1 - const/4 v0, 0x0 - return v0 -.end method - -# Test a case when one entering TryBoundary is dead but the rest of the try -# block remains live. - -## CHECK-START: int TestCase.testDeadEntry(int, int, int, int) dead_code_elimination_final (before) -## CHECK: Add - -## CHECK-START: int TestCase.testDeadEntry(int, int, int, int) dead_code_elimination_final (before) -## CHECK: TryBoundary kind:entry -## CHECK: TryBoundary kind:entry -## CHECK-NOT: TryBoundary kind:entry - -## CHECK-START: int TestCase.testDeadEntry(int, int, int, int) dead_code_elimination_final (after) -## CHECK-NOT: Add - -## CHECK-START: int TestCase.testDeadEntry(int, int, int, int) dead_code_elimination_final (after) -## CHECK: TryBoundary kind:entry -## CHECK-NOT: TryBoundary kind:entry - -.method public static testDeadEntry(IIII)I - .registers 5 - - invoke-static {}, LTestCase;->$inline$False()Z - move-result v0 - - if-eqz v0, :else - - add-int/2addr p0, p1 - - :try_start - div-int/2addr p0, p2 - - :else - div-int/2addr p0, p3 - :try_end - .catchall {:try_start .. :try_end} :catch_all - - :return - return p0 - - :catch_all - const/4 p0, -0x1 - goto :return - -.end method - -# Test a case when one exiting TryBoundary is dead but the rest of the try -# block remains live. - -## CHECK-START: int TestCase.testDeadExit(int, int, int, int) dead_code_elimination_final (before) -## CHECK: Add - -## CHECK-START: int TestCase.testDeadExit(int, int, int, int) dead_code_elimination_final (before) -## CHECK: TryBoundary kind:exit -## CHECK: TryBoundary kind:exit -## CHECK-NOT: TryBoundary kind:exit - -## CHECK-START: int TestCase.testDeadExit(int, int, int, int) dead_code_elimination_final (after) -## CHECK-NOT: Add - -## CHECK-START: int TestCase.testDeadExit(int, int, int, int) dead_code_elimination_final (after) -## CHECK: TryBoundary kind:exit -## CHECK-NOT: TryBoundary kind:exit - -.method public static testDeadExit(IIII)I - .registers 5 - - invoke-static {}, LTestCase;->$inline$False()Z - move-result v0 - - :try_start - div-int/2addr p0, p2 - - if-nez v0, :else - - div-int/2addr p0, p3 - goto :return - :try_end - .catchall {:try_start .. :try_end} :catch_all - - :else - add-int/2addr p0, p1 - - :return - return p0 - - :catch_all - const/4 p0, -0x1 - goto :return - -.end method - -# Test that a catch block remains live and consistent if some of try blocks -# throwing into it are removed. - -## CHECK-START: int TestCase.testOneTryBlockDead(int, int, int, int) dead_code_elimination_final (before) -## CHECK: TryBoundary kind:entry -## CHECK: TryBoundary kind:entry -## CHECK-NOT: TryBoundary kind:entry - -## CHECK-START: int TestCase.testOneTryBlockDead(int, int, int, int) dead_code_elimination_final (before) -## CHECK: TryBoundary kind:exit -## CHECK: TryBoundary kind:exit -## CHECK-NOT: TryBoundary kind:exit - -## CHECK-START: int TestCase.testOneTryBlockDead(int, int, int, int) dead_code_elimination_final (after) -## CHECK: TryBoundary kind:entry -## CHECK-NOT: TryBoundary kind:entry - -## CHECK-START: int TestCase.testOneTryBlockDead(int, int, int, int) dead_code_elimination_final (after) -## CHECK: TryBoundary kind:exit -## CHECK-NOT: TryBoundary kind:exit - -.method public static testOneTryBlockDead(IIII)I - .registers 5 - - invoke-static {}, LTestCase;->$inline$False()Z - move-result v0 - - :try_start_1 - div-int/2addr p0, p2 - :try_end_1 - .catchall {:try_start_1 .. :try_end_1} :catch_all - - if-eqz v0, :return - - :try_start_2 - div-int/2addr p0, p3 - :try_end_2 - .catchall {:try_start_2 .. :try_end_2} :catch_all - - :return - return p0 - - :catch_all - const/4 p0, -0x1 - goto :return - -.end method - -# Test that try block membership is recomputed. In this test case, the try entry -# stored with the merge block gets deleted and SSAChecker would fail if it was -# not replaced with the try entry from the live branch. - -.method public static testRecomputeTryMembership(IIII)I - .registers 5 - - invoke-static {}, LTestCase;->$inline$False()Z - move-result v0 - - if-eqz v0, :else - - # Dead branch - :try_start - div-int/2addr p0, p1 - goto :merge - - # Live branch - :else - div-int/2addr p0, p2 - - # Merge block. Make complex so it does not get merged with the live branch. - :merge - div-int/2addr p0, p3 - if-eqz p0, :else2 - div-int/2addr p0, p3 - :else2 - :try_end - .catchall {:try_start .. :try_end} :catch_all - - :return - return p0 - - :catch_all - const/4 p0, -0x1 - goto :return - -.end method diff --git a/test/543-checker-dce-trycatch/src/Main.java b/test/543-checker-dce-trycatch/src/Main.java deleted file mode 100644 index 6e73d0dbd..000000000 --- a/test/543-checker-dce-trycatch/src/Main.java +++ /dev/null @@ -1,66 +0,0 @@ -/* - * Copyright (C) 2015 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. - */ - -public class Main { - - // Workaround for b/18051191. - class InnerClass {} - - static boolean $inline$False() { return false; } - - // DCE should only merge blocks where the first ends with a Goto. - // SSAChecker will fail if the following Throw->TryBoundary blocks are merged. - public static void doNotMergeThrow(String str) { - try { - throw new Exception(str); - } catch (Exception ex) { - return; - } - } - - // Test deletion of all try/catch blocks. Multiple catch blocks test deletion - // where TryBoundary still has exception handler successors after having removed - // some already. - - /// CHECK-START: void Main.testDeadTryCatch(boolean) dead_code_elimination_final (after) - /// CHECK-NOT: TryBoundary - - /// CHECK-START: void Main.testDeadTryCatch(boolean) dead_code_elimination_final (after) - /// CHECK: begin_block - /// CHECK: begin_block - /// CHECK: begin_block - /// CHECK-NOT: begin_block - - public static void testDeadTryCatch(boolean val) { - if ($inline$False()) { - try { - if (val) { - throw new ArithmeticException(); - } else { - throw new ArrayIndexOutOfBoundsException(); - } - } catch (ArithmeticException ex) { - System.out.println("Unexpected AE catch"); - } catch (ArrayIndexOutOfBoundsException ex) { - System.out.println("Unexpected AIIOB catch"); - } - } - } - - public static void main(String[] args) { - - } -} \ No newline at end of file -- 2.11.0