From 04ff4e8463ac68752638d305eeb84b457fd8289c Mon Sep 17 00:00:00 2001 From: David Brazdil Date: Thu, 10 Dec 2015 13:54:52 +0000 Subject: [PATCH] ART: Fix bug in DCE not removing phis from catch phi uses Due to the missing edges between throwing instructions and catch phis DCE needs to manually remove dead instructions from catch phi users, being overly conservative if the inputs are not in the dead blocks. DCE used to do this for normal instructions, but it needs to do the same for phis. Change-Id: I7edfcb84ec6ff7303945d5d5cd436b1d1e95df2a --- compiler/optimizing/nodes.cc | 34 ++++++++----- test/543-checker-dce-trycatch/smali/TestCase.smali | 56 ++++++++++++++-------- 2 files changed, 57 insertions(+), 33 deletions(-) diff --git a/compiler/optimizing/nodes.cc b/compiler/optimizing/nodes.cc index 461be2588..926bc156c 100644 --- a/compiler/optimizing/nodes.cc +++ b/compiler/optimizing/nodes.cc @@ -1460,6 +1460,24 @@ void HInstructionList::Add(const HInstructionList& instruction_list) { } } +// Should be called on instructions in a dead block in post order. This method +// assumes `insn` has been removed from all users with the exception of catch +// phis because of missing exceptional edges in the graph. It removes the +// instruction from catch phi uses, together with inputs of other catch phis in +// the catch block at the same index, as these must be dead too. +static void RemoveUsesOfDeadInstruction(HInstruction* insn) { + DCHECK(!insn->HasEnvironmentUses()); + while (insn->HasNonEnvironmentUses()) { + HUseListNode* use = insn->GetUses().GetFirst(); + size_t use_index = use->GetIndex(); + HBasicBlock* user_block = use->GetUser()->GetBlock(); + DCHECK(use->GetUser()->IsPhi() && user_block->IsCatchBlock()); + for (HInstructionIterator phi_it(user_block->GetPhis()); !phi_it.Done(); phi_it.Advance()) { + phi_it.Current()->AsPhi()->RemoveInputAt(use_index); + } + } +} + void HBasicBlock::DisconnectAndDelete() { // Dominators must be removed after all the blocks they dominate. This way // a loop header is removed last, a requirement for correct loop information @@ -1562,21 +1580,13 @@ void HBasicBlock::DisconnectAndDelete() { // graph will always remain consistent. for (HBackwardInstructionIterator it(GetInstructions()); !it.Done(); it.Advance()) { HInstruction* insn = it.Current(); - while (insn->HasUses()) { - DCHECK(IsTryBlock()); - HUseListNode* use = insn->GetUses().GetFirst(); - size_t use_index = use->GetIndex(); - HBasicBlock* user_block = use->GetUser()->GetBlock(); - DCHECK(use->GetUser()->IsPhi() && user_block->IsCatchBlock()); - for (HInstructionIterator phi_it(user_block->GetPhis()); !phi_it.Done(); phi_it.Advance()) { - phi_it.Current()->AsPhi()->RemoveInputAt(use_index); - } - } - + RemoveUsesOfDeadInstruction(insn); RemoveInstruction(insn); } for (HInstructionIterator it(GetPhis()); !it.Done(); it.Advance()) { - RemovePhi(it.Current()->AsPhi()); + HPhi* insn = it.Current()->AsPhi(); + RemoveUsesOfDeadInstruction(insn); + RemovePhi(insn); } // Disconnect from the dominator. diff --git a/test/543-checker-dce-trycatch/smali/TestCase.smali b/test/543-checker-dce-trycatch/smali/TestCase.smali index 44e907d80..1756fa4a9 100644 --- a/test/543-checker-dce-trycatch/smali/TestCase.smali +++ b/test/543-checker-dce-trycatch/smali/TestCase.smali @@ -202,27 +202,35 @@ # Test that DCE removes catch phi uses of instructions defined in dead try blocks. ## CHECK-START: int TestCase.testCatchPhiInputs_DefinedInTryBlock(int, int, int, int) dead_code_elimination_final (before) -## CHECK-DAG: <> ParameterValue -## CHECK-DAG: <> ParameterValue -## CHECK-DAG: <> IntConstant 10 -## CHECK-DAG: <> IntConstant 11 -## CHECK-DAG: <> IntConstant 12 -## CHECK-DAG: <> IntConstant 13 -## CHECK-DAG: <> IntConstant 14 -## CHECK-DAG: <> Add [<>,<>] -## CHECK-DAG: Phi [<>,<>,<>] reg:1 is_catch_phi:true -## CHECK-DAG: Phi [<>,<>,<>] reg:2 is_catch_phi:true +## CHECK-DAG: <> ParameterValue +## CHECK-DAG: <> ParameterValue +## CHECK-DAG: <> IntConstant 10 +## CHECK-DAG: <> IntConstant 11 +## CHECK-DAG: <> IntConstant 12 +## CHECK-DAG: <> IntConstant 13 +## CHECK-DAG: <> IntConstant 14 +## CHECK-DAG: <> IntConstant 15 +## CHECK-DAG: <> IntConstant 16 +## CHECK-DAG: <> IntConstant 17 +## CHECK-DAG: <> Add [<>,<>] +## CHECK-DAG: <> Phi [<>,<>] reg:3 is_catch_phi:false +## CHECK-DAG: Phi [<>,<>,<>] reg:1 is_catch_phi:true +## CHECK-DAG: Phi [<>,<>,<>] reg:2 is_catch_phi:true +## CHECK-DAG: Phi [<>,<>,<>] reg:3 is_catch_phi:true ## CHECK-START: int TestCase.testCatchPhiInputs_DefinedInTryBlock(int, int, int, int) dead_code_elimination_final (after) -## CHECK-DAG: <> IntConstant 11 -## CHECK-DAG: <> IntConstant 12 -## CHECK-DAG: <> IntConstant 13 -## CHECK-DAG: <> IntConstant 14 -## CHECK-DAG: Phi [<>,<>] reg:1 is_catch_phi:true -## CHECK-DAG: Phi [<>,<>] reg:2 is_catch_phi:true +## CHECK-DAG: <> IntConstant 11 +## CHECK-DAG: <> IntConstant 12 +## CHECK-DAG: <> IntConstant 13 +## CHECK-DAG: <> IntConstant 14 +## CHECK-DAG: <> IntConstant 16 +## CHECK-DAG: <> IntConstant 17 +## CHECK-DAG: Phi [<>,<>] reg:1 is_catch_phi:true +## CHECK-DAG: Phi [<>,<>] reg:2 is_catch_phi:true +## CHECK-DAG: Phi [<>,<>] reg:3 is_catch_phi:true .method public static testCatchPhiInputs_DefinedInTryBlock(IIII)I - .registers 7 + .registers 8 invoke-static {}, LTestCase;->$inline$False()Z move-result v0 @@ -232,17 +240,24 @@ shr-int/2addr p2, p3 :try_start - const v1, 0xa # dead catch phi input, defined in entry block - add-int v2, p0, p1 # dead catch phi input, defined in the dead block + const v1, 0xa # dead catch phi input, defined in entry block (HInstruction) + add-int v2, p0, p1 # dead catch phi input, defined in the dead block (HInstruction) + move v3, v2 + if-eqz v3, :define_phi + const v3, 0xf + :define_phi + # v3 = Phi [Add, 0xf] # dead catch phi input, defined in the dead block (HPhi) div-int/2addr p0, v2 :else const v1, 0xb # live catch phi input const v2, 0xc # live catch phi input + const v3, 0x10 # live catch phi input div-int/2addr p0, p3 const v1, 0xd # live catch phi input const v2, 0xe # live catch phi input + const v3, 0x11 # live catch phi input div-int/2addr p0, p1 :try_end .catchall {:try_start .. :try_end} :catch_all @@ -252,6 +267,7 @@ :catch_all sub-int p0, v1, v2 # use catch phi values + sub-int p0, p0, v3 # use catch phi values goto :return .end method @@ -260,8 +276,6 @@ # dead try blocks. ## CHECK-START: int TestCase.testCatchPhiInputs_DefinedOutsideTryBlock(int, int, int, int) dead_code_elimination_final (before) -## CHECK-DAG: <> ParameterValue -## CHECK-DAG: <> ParameterValue ## CHECK-DAG: <> IntConstant 10 ## CHECK-DAG: <> IntConstant 11 ## CHECK-DAG: <> IntConstant 12 -- 2.11.0