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

1 files changed, 20 insertions, 2 deletions

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<kArenaAllocGvn> {
   // 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<kArenaAllocGvn> {
       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.