Don't apply instance-of peephole when vDest == vSrc.

Bug: 9284898.
Also statistic to show check-cast ellision use.
Fix bug where the check-cast ellision was using wrong dex pc.
Avoid a use of DecodedInstruction.
Other formatting clean-up.

Change-Id: Ibf67941a24148b615896d0be6f2f29ce5034e53a

5 files changed, 85 insertions, 48 deletions

diff --git a/src/compiler/dex/quick/gen_common.cc b/src/compiler/dex/quick/gen_common.cc
index 69235273f8..12bdddd8d0 100644
--- a/src/compiler/dex/quick/gen_common.cc
+++ b/src/compiler/dex/quick/gen_common.cc
@@ -974,7 +974,8 @@ void Mir2Lir::GenCheckCast(uint32_t insn_idx, uint32_t type_idx, RegLocation rl_
         cu->GetDexFile(),
         cu->GetDexMethodIndex());
 
-  if (verifier::MethodVerifier::IsSafeCast(mr, insn_idx)) {
+  if (cu_->compiler_driver->IsSafeCast(mr, insn_idx)) {
+    // Verifier type analysis proved this check cast would never cause an exception.
     return;
   }
   FlushAllRegs();
diff --git a/src/compiler/driver/compiler_driver.cc b/src/compiler/driver/compiler_driver.cc
index 2783e2e13e..3dddc751be 100644
--- a/src/compiler/driver/compiler_driver.cc
+++ b/src/compiler/driver/compiler_driver.cc
@@ -72,7 +72,8 @@ class AOTCompilationStats {
         resolved_types_(0), unresolved_types_(0),
         resolved_instance_fields_(0), unresolved_instance_fields_(0),
         resolved_local_static_fields_(0), resolved_static_fields_(0), unresolved_static_fields_(0),
-        type_based_devirtualization_(0) {
+        type_based_devirtualization_(0),
+        safe_casts_(0), not_safe_casts_(0) {
     for (size_t i = 0; i <= kMaxInvokeType; i++) {
       resolved_methods_[i] = 0;
       unresolved_methods_[i] = 0;
@@ -91,8 +92,14 @@ class AOTCompilationStats {
              "static fields resolved");
     DumpStat(resolved_local_static_fields_, resolved_static_fields_ + unresolved_static_fields_,
              "static fields local to a class");
-    DumpStat(type_based_devirtualization_,virtual_made_direct_[kInterface] + virtual_made_direct_[kVirtual]
-             - type_based_devirtualization_, "sharpened calls based on type information");
+    DumpStat(safe_casts_, not_safe_casts_, "check-casts removed based on type information");
+    // Note, the code below subtracts the stat value so that when added to the stat value we have
+    // 100% of samples. TODO: clean this up.
+    DumpStat(type_based_devirtualization_,
+             resolved_methods_[kVirtual] + unresolved_methods_[kVirtual] +
+             resolved_methods_[kInterface] + unresolved_methods_[kInterface] -
+             type_based_devirtualization_,
+             "virtual/interface calls made direct based on type information");
 
     for (size_t i = 0; i <= kMaxInvokeType; i++) {
       std::ostringstream oss;
@@ -227,6 +234,18 @@ class AOTCompilationStats {
     direct_methods_to_boot_[type]++;
   }
 
+  // A check-cast could be eliminated due to verifier type analysis.
+  void SafeCast() {
+    STATS_LOCK();
+    safe_casts_++;
+  }
+
+  // A check-cast couldn't be eliminated due to verifier type analysis.
+  void NotASafeCast() {
+    STATS_LOCK();
+    not_safe_casts_++;
+  }
+
  private:
   Mutex stats_lock_;
 
@@ -254,6 +273,9 @@ class AOTCompilationStats {
   size_t direct_calls_to_boot_[kMaxInvokeType + 1];
   size_t direct_methods_to_boot_[kMaxInvokeType + 1];
 
+  size_t safe_casts_;
+  size_t not_safe_casts_;
+
   DISALLOW_COPY_AND_ASSIGN(AOTCompilationStats);
 };
 
@@ -1014,6 +1036,17 @@ bool CompilerDriver::ComputeInvokeInfo(const DexCompilationUnit* mUnit, const ui
   return false;  // Incomplete knowledge needs slow path.
 }
 
+bool CompilerDriver::IsSafeCast(const MethodReference& mr, uint32_t dex_pc) {
+  bool result = verifier::MethodVerifier::IsSafeCast(mr, dex_pc);
+  if (result) {
+    stats_->SafeCast();
+  } else {
+    stats_->NotASafeCast();
+  }
+  return result;
+}
+
+
 void CompilerDriver::AddCodePatch(const DexFile* dex_file,
                             uint32_t referrer_method_idx,
                             InvokeType referrer_invoke_type,
diff --git a/src/compiler/driver/compiler_driver.h b/src/compiler/driver/compiler_driver.h
index c1e449e6ed..1b5bd0d84c 100644
--- a/src/compiler/driver/compiler_driver.h
+++ b/src/compiler/driver/compiler_driver.h
@@ -165,6 +165,8 @@ class CompilerDriver {
                          uintptr_t& direct_code, uintptr_t& direct_method, bool update_stats)
       LOCKS_EXCLUDED(Locks::mutator_lock_);
 
+  bool IsSafeCast(const MethodReference& mr, uint32_t dex_pc);
+
   // Record patch information for later fix up.
   void AddCodePatch(const DexFile* dex_file,
                     uint32_t referrer_method_idx,
diff --git a/src/verifier/method_verifier.cc b/src/verifier/method_verifier.cc
index 519de80db8..9d4b902cf0 100644
--- a/src/verifier/method_verifier.cc
+++ b/src/verifier/method_verifier.cc
@@ -1777,31 +1777,33 @@ bool MethodVerifier::CodeFlowVerifyInstruction(uint32_t* start_guess) {
 
       /* Check for peep-hole pattern of:
        *    ...;
-       *    instance-of vX, vO, T;
-       *    ifXXX vX, b ;
+       *    instance-of vX, vY, T;
+       *    ifXXX vX, label ;
        *    ...;
-       * b: INST;
+       * label:
        *    ...;
-       * and sharpen the type for either the fall-through or the branch case.
+       * and sharpen the type of vY to be type T.
+       * Note, this pattern can't be if:
+       *  - if there are other branches to this branch,
+       *  - when vX == vY.
        */
-      if (!CurrentInsnFlags()->IsBranchTarget()) {
-        if ((Instruction::INSTANCE_OF == prev_inst->Opcode())
-            && (inst->VRegA_21t() == prev_inst->VRegA_22c())) {
-          // Check that the we are not attempting conversion to interface types,
-          // which is not done because of the multiple inheritance implications.
-          const RegType& cast_type =
-                    ResolveClassAndCheckAccess(prev_inst->VRegC_22c());
-
-          if(!cast_type.IsUnresolvedTypes() && !cast_type.GetClass()->IsInterface()) {
-            if (inst->Opcode() == Instruction::IF_EQZ) {
-              fallthrough_line.reset(new RegisterLine(code_item_->registers_size_, this));
-              fallthrough_line->CopyFromLine(work_line_.get());
-              fallthrough_line->SetRegisterType(prev_inst->VRegB_22c(), cast_type);
-            } else {
-              branch_line.reset(new RegisterLine(code_item_->registers_size_, this));
-              branch_line->CopyFromLine(work_line_.get());
-              branch_line->SetRegisterType(prev_inst->VRegB_22c(), cast_type);
-            }
+      if (!CurrentInsnFlags()->IsBranchTarget() &&
+          (Instruction::INSTANCE_OF == prev_inst->Opcode()) &&
+          (inst->VRegA_21t() == prev_inst->VRegA_22c()) &&
+          (prev_inst->VRegA_22c() != prev_inst->VRegB_22c())) {
+        // Check that the we are not attempting conversion to interface types,
+        // which is not done because of the multiple inheritance implications.
+        const RegType& cast_type = ResolveClassAndCheckAccess(prev_inst->VRegC_22c());
+
+        if(!cast_type.IsUnresolvedTypes() && !cast_type.GetClass()->IsInterface()) {
+          if (inst->Opcode() == Instruction::IF_EQZ) {
+            fallthrough_line.reset(new RegisterLine(code_item_->registers_size_, this));
+            fallthrough_line->CopyFromLine(work_line_.get());
+            fallthrough_line->SetRegisterType(prev_inst->VRegB_22c(), cast_type);
+          } else {
+            branch_line.reset(new RegisterLine(code_item_->registers_size_, this));
+            branch_line->CopyFromLine(work_line_.get());
+            branch_line->SetRegisterType(prev_inst->VRegB_22c(), cast_type);
           }
         }
       }
@@ -3304,24 +3306,22 @@ MethodVerifier::MethodSafeCastSet* MethodVerifier::GenerateSafeCastSet() {
     return NULL;
   }
   UniquePtr<MethodSafeCastSet> mscs;
-  uint32_t dex_pc = 0;
   const Instruction* inst = Instruction::At(code_item_->insns_);
   const Instruction* end = Instruction::At(code_item_->insns_ +
-      code_item_->insns_size_in_code_units_);
+                                           code_item_->insns_size_in_code_units_);
 
   for (; inst < end; inst = inst->Next()) {
-    if( Instruction::CHECK_CAST != inst->Opcode() )
+    if (Instruction::CHECK_CAST != inst->Opcode()) {
       continue;
-
+    }
+    uint32_t dex_pc = inst->GetDexPc(code_item_->insns_);
     RegisterLine* line = reg_table_.GetLine(dex_pc);
-    DecodedInstruction dec_insn(inst);
-    const RegType& reg_type(line->GetRegisterType(dec_insn.vA));
-    const RegType& cast_type = ResolveClassAndCheckAccess(dec_insn.vB);
+    const RegType& reg_type(line->GetRegisterType(inst->VRegA_21c()));
+    const RegType& cast_type = ResolveClassAndCheckAccess(inst->VRegB_21c());
     if (cast_type.IsAssignableFrom(reg_type)) {
       if (mscs.get() == NULL) {
         mscs.reset(new MethodSafeCastSet());
       }
-      uint32_t dex_pc = inst->GetDexPc(code_item_->insns_);
       mscs->insert(dex_pc);
     }
   }
@@ -3511,7 +3511,8 @@ void MethodVerifier::SetDexGcMap(CompilerDriver::MethodReference ref,
 }
 
 
-void  MethodVerifier::SetSafeCastMap(CompilerDriver::MethodReference ref, const MethodSafeCastSet* cast_set) {
+void  MethodVerifier::SetSafeCastMap(CompilerDriver::MethodReference ref,
+                                     const MethodSafeCastSet* cast_set) {
   MutexLock mu(Thread::Current(), *safecast_map_lock_);
   SafeCastMap::iterator it = safecast_map_->find(ref);
   if (it != safecast_map_->end()) {
diff --git a/src/verifier/reg_type.cc b/src/verifier/reg_type.cc
index e738e80514..55ebcdbff5 100644
--- a/src/verifier/reg_type.cc
+++ b/src/verifier/reg_type.cc
@@ -739,19 +739,19 @@ bool RegType::IsAssignableFrom(const RegType& src) const {
       } else if (IsJavaLangObject()) {
         return true;  // all reference types can be assigned to Object
       } else if (!IsUnresolvedTypes() && GetClass()->IsInterface()) {
-          return true;  // We allow assignment to any interface, see comment in ClassJoin
-        } else if (IsJavaLangObjectArray()) {
-          return src.IsObjectArrayTypes();  // All reference arrays may be assigned to Object[]
-        } else if (!IsUnresolvedTypes() && !src.IsUnresolvedTypes() &&
-                   GetClass()->IsAssignableFrom(src.GetClass())) {
-          // We're assignable from the Class point-of-view
-          return true;
-        } else if (IsUnresolvedTypes()) {
-          // Unresolved types are only assignable for null, Object and equality.
-          return (src.IsZero() || src.IsJavaLangObject());
-        } else {
-          return false;
-        }
+        return true;  // We allow assignment to any interface, see comment in ClassJoin
+      } else if (IsJavaLangObjectArray()) {
+        return src.IsObjectArrayTypes();  // All reference arrays may be assigned to Object[]
+      } else if (!IsUnresolvedTypes() && !src.IsUnresolvedTypes() &&
+          GetClass()->IsAssignableFrom(src.GetClass())) {
+        // We're assignable from the Class point-of-view
+        return true;
+      } else if (IsUnresolvedTypes()) {
+        // Unresolved types are only assignable for null, Object and equality.
+        return (src.IsZero() || src.IsJavaLangObject());
+      } else {
+        return false;
+      }
     }
   }
 }