Quick: Fix wide Phi detection in GVN, clean up INVOKEs.

The detection of a wide Phi has been incorrectly looking at
the current LVN's wide sreg value map but we only intersect
live values and thus very often lose the information. This
results in failure to identify identical values, i.e.
potential missed optimizations. It also caused the bloating
of the global value map with values we would not use.

Rewrite the wide Phi detection to use the first merged LVN's
notion of wide sreg. For this to work we also need to use
the method's shorty to mark wide arguments.

Also clean up INVOKEs' processing to avoid another source
of bloating the global value map.

Bug: 16398693
Change-Id: I76718af7d62a8c6883ef43e4f47058f7eaf479e1

1 files changed, 58 insertions, 18 deletions

diff --git a/compiler/dex/local_value_numbering.cc b/compiler/dex/local_value_numbering.cc
index 8b7ae20b95..5456f4d0a1 100644
--- a/compiler/dex/local_value_numbering.cc
+++ b/compiler/dex/local_value_numbering.cc
@@ -374,6 +374,12 @@ void LocalValueNumbering::MergeOne(const LocalValueNumbering& other, MergeType m
   range_checked_ = other.range_checked_;
   null_checked_ = other.null_checked_;
 
+  const BasicBlock* pred_bb = gvn_->GetBasicBlock(other.Id());
+  if (GlobalValueNumbering::HasNullCheckLastInsn(pred_bb, Id())) {
+    int s_reg = pred_bb->last_mir_insn->ssa_rep->uses[0];
+    null_checked_.insert(other.GetOperandValue(s_reg));
+  }
+
   if (merge_type == kCatchMerge) {
     // Memory is clobbered. Use new memory version and don't merge aliasing locations.
     global_memory_version_ = NewMemoryVersion(&merge_new_memory_version_);
@@ -465,10 +471,7 @@ void LocalValueNumbering::PruneNonAliasingRefsForCatch() {
     DCHECK(mir != nullptr);
     // Only INVOKEs can leak and clobber non-aliasing references if they throw.
     if ((mir->dalvikInsn.FlagsOf() & Instruction::kInvoke) != 0) {
-      for (uint16_t i = 0u; i != mir->ssa_rep->num_uses; ++i) {
-        uint16_t value_name = lvn->GetOperandValue(mir->ssa_rep->uses[i]);
-        non_aliasing_refs_.erase(value_name);
-      }
+      HandleInvokeArgs(mir, lvn);
     }
   }
 }
@@ -681,7 +684,7 @@ void LocalValueNumbering::MergeNullChecked() {
   const BasicBlock* least_entries_bb = gvn_->GetBasicBlock(least_entries_lvn->Id());
   if (gvn_->HasNullCheckLastInsn(least_entries_bb, id_)) {
     int s_reg = least_entries_bb->last_mir_insn->ssa_rep->uses[0];
-    uint32_t value_name = least_entries_lvn->GetSRegValueName(s_reg);
+    uint32_t value_name = least_entries_lvn->GetOperandValue(s_reg);
     merge_names_.clear();
     merge_names_.resize(gvn_->merge_lvns_.size(), value_name);
     if (gvn_->NullCheckedInAllPredecessors(merge_names_)) {
@@ -953,6 +956,26 @@ void LocalValueNumbering::Merge(MergeType merge_type) {
                 AliasingArrayVersions>>();
 }
 
+void LocalValueNumbering::PrepareEntryBlock() {
+  uint32_t vreg = gvn_->GetMirGraph()->GetFirstInVR();
+  CompilationUnit* cu = gvn_->GetCompilationUnit();
+  const char* shorty = cu->shorty;
+  ++shorty;  // Skip return value.
+  if ((cu->access_flags & kAccStatic) == 0) {
+    // If non-static method, mark "this" as non-null
+    uint16_t value_name = GetOperandValue(vreg);
+    ++vreg;
+    null_checked_.insert(value_name);
+  }
+  for ( ; *shorty != 0; ++shorty, ++vreg) {
+    if (*shorty == 'J' || *shorty == 'D') {
+      uint16_t value_name = GetOperandValueWide(vreg);
+      SetOperandValueWide(vreg, value_name);
+      ++vreg;
+    }
+  }
+}
+
 uint16_t LocalValueNumbering::MarkNonAliasingNonNull(MIR* mir) {
   uint16_t res = GetOperandValue(mir->ssa_rep->defs[0]);
   DCHECK(null_checked_.find(res) == null_checked_.end());
@@ -1039,12 +1062,30 @@ void LocalValueNumbering::HandleEscapingRef(uint16_t base) {
   }
 }
 
+void LocalValueNumbering::HandleInvokeArgs(const MIR* mir, const LocalValueNumbering* mir_lvn) {
+  const int32_t* uses = mir->ssa_rep->uses;
+  const int32_t* uses_end = uses + mir->ssa_rep->num_uses;
+  while (uses != uses_end) {
+    uint16_t sreg = *uses;
+    ++uses;
+    // Avoid LookupValue() so that we don't store new values in the global value map.
+    auto local_it = mir_lvn->sreg_value_map_.find(sreg);
+    if (local_it != mir_lvn->sreg_value_map_.end()) {
+      non_aliasing_refs_.erase(local_it->second);
+    } else {
+      uint16_t value_name = gvn_->FindValue(kNoValue, sreg, kNoValue, kNoValue);
+      if (value_name != kNoValue) {
+        non_aliasing_refs_.erase(value_name);
+      }
+    }
+  }
+}
+
 uint16_t LocalValueNumbering::HandlePhi(MIR* mir) {
   if (gvn_->merge_lvns_.empty()) {
     // Running LVN without a full GVN?
     return kNoValue;
   }
-  int16_t num_uses = mir->ssa_rep->num_uses;
   int32_t* uses = mir->ssa_rep->uses;
   // Try to find out if this is merging wide regs.
   if (mir->ssa_rep->defs[0] != 0 &&
@@ -1052,18 +1093,20 @@ uint16_t LocalValueNumbering::HandlePhi(MIR* mir) {
     // This is the high part of a wide reg. Ignore the Phi.
     return kNoValue;
   }
-  bool wide = false;
-  for (int16_t i = 0; i != num_uses; ++i) {
-    if (sreg_wide_value_map_.count(uses[i]) != 0u) {
-      wide = true;
-      break;
-    }
+  BasicBlockId* incoming = mir->meta.phi_incoming;
+  int16_t pos = 0;
+  // Check if we're merging a wide value based on the first merged LVN.
+  const LocalValueNumbering* first_lvn = gvn_->merge_lvns_[0];
+  DCHECK_LT(pos, mir->ssa_rep->num_uses);
+  while (incoming[pos] != first_lvn->Id()) {
+    ++pos;
+    DCHECK_LT(pos, mir->ssa_rep->num_uses);
   }
+  int first_s_reg = uses[pos];
+  bool wide = (first_lvn->sreg_wide_value_map_.count(first_s_reg) != 0u);
   // Iterate over *merge_lvns_ and skip incoming sregs for BBs without associated LVN.
   uint16_t value_name = kNoValue;
   merge_names_.clear();
-  BasicBlockId* incoming = mir->meta.phi_incoming;
-  int16_t pos = 0;
   bool same_values = true;
   for (const LocalValueNumbering* lvn : gvn_->merge_lvns_) {
     DCHECK_LT(pos, mir->ssa_rep->num_uses);
@@ -1468,10 +1511,7 @@ uint16_t LocalValueNumbering::GetValueNumber(MIR* mir) {
     case Instruction::INVOKE_STATIC:
     case Instruction::INVOKE_STATIC_RANGE:
       // Make ref args aliasing.
-      for (size_t i = 0u, count = mir->ssa_rep->num_uses; i != count; ++i) {
-        uint16_t reg = GetOperandValue(mir->ssa_rep->uses[i]);
-        non_aliasing_refs_.erase(reg);
-      }
+      HandleInvokeArgs(mir, this);
       HandleInvokeOrClInitOrAcquireOp(mir);
       break;