From 289bd1cccdb3aa37e2d129980f5c151f52f84897 Mon Sep 17 00:00:00 2001 From: Alex Light Date: Fri, 3 Apr 2020 15:42:30 -0700 Subject: Make GVN handle HDeoptimize better The GVN system didn't handle the deoptimization very well since deoptimize is a predicated operation. This means that HDeoptimize needs to prevent some code motion but if it isn't taken the operation has no effect. This confused the GVN system into thinking that it cannot merge deoptimizations. To fix this we special cased the side effects GVN considers deoptimizations to have. Test: ./test.py --host Change-Id: Ic79d975f9ae584a07026647cee2768ed1105e5a9 --- compiler/optimizing/gvn.cc | 22 ++++++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-) (limited to 'compiler/optimizing') diff --git a/compiler/optimizing/gvn.cc b/compiler/optimizing/gvn.cc index e8460a843f..c7cd223b51 100644 --- a/compiler/optimizing/gvn.cc +++ b/compiler/optimizing/gvn.cc @@ -126,7 +126,7 @@ class ValueSet : public ArenaObject { // Removes all instructions in the set affected by the given side effects. void Kill(SideEffects side_effects) { DeleteAllImpureWhich([side_effects](Node* node) { - return node->GetInstruction()->GetSideEffects().MayDependOn(side_effects); + return node->GetSideEffects().MayDependOn(side_effects); }); } @@ -197,6 +197,21 @@ class ValueSet : public ArenaObject { return new (allocator) Node(instruction_, hash_code_, new_next); } + SideEffects GetSideEffects() const { + // Deoptimize is a weird instruction since it's predicated and + // never-return. Its side-effects are to prevent the splitting of dex + // instructions across it (which could cause inconsistencies once we begin + // interpreting again). In the context of GVN the 'perform-deopt' branch is not + // relevant and we only need to care about the no-op case, in which case there are + // no side-effects. By doing this we are able to eliminate redundant (i.e. + // dominated deopts with GVNd conditions) deoptimizations. + if (instruction_->IsDeoptimize()) { + return SideEffects::None(); + } else { + return instruction_->GetSideEffects(); + } + } + private: HInstruction* const instruction_; const size_t hash_code_; @@ -479,7 +494,10 @@ void GlobalValueNumberer::VisitBasicBlock(HBasicBlock* block) { // // BoundType is a special case example of an instruction which shouldn't be moved but can be // GVN'ed. - if (current->CanBeMoved() || current->IsBoundType()) { + // + // Deoptimize is a special case since even though we don't want to move it we can still remove + // it for GVN. + if (current->CanBeMoved() || current->IsBoundType() || current->IsDeoptimize()) { if (current->IsBinaryOperation() && current->AsBinaryOperation()->IsCommutative()) { // For commutative ops, (x op y) will be treated the same as (y op x) // after fixed ordering. -- cgit v1.2.3-59-g8ed1b