Revert "ART: Removes SuspendCheck for plain loops with a low trip count."

This reverts commit 3de02fb67de386368c9fe39ab5a0133afcf1d785.

Reason for revert: b/201413752

Change-Id: I8a3a56cfb81fa88f49992615696260dfbc79bb67

5 files changed, 12 insertions, 83 deletions

diff --git a/compiler/optimizing/code_generator.cc b/compiler/optimizing/code_generator.cc
index cabf94991d..6b3ae419c5 100644
--- a/compiler/optimizing/code_generator.cc
+++ b/compiler/optimizing/code_generator.cc
@@ -1118,7 +1118,7 @@ static void CheckLoopEntriesCanBeUsedForOsr(const HGraph& graph,
   for (HBasicBlock* block : graph.GetReversePostOrder()) {
     if (block->IsLoopHeader()) {
       HSuspendCheck* suspend_check = block->GetLoopInformation()->GetSuspendCheck();
-      if (suspend_check != nullptr && !suspend_check->GetEnvironment()->IsFromInlinedInvoke()) {
+      if (!suspend_check->GetEnvironment()->IsFromInlinedInvoke()) {
         loop_headers.push_back(suspend_check);
       }
     }
diff --git a/compiler/optimizing/graph_checker.cc b/compiler/optimizing/graph_checker.cc
index 55a505c5ea..d1769cea0d 100644
--- a/compiler/optimizing/graph_checker.cc
+++ b/compiler/optimizing/graph_checker.cc
@@ -674,14 +674,13 @@ void GraphChecker::HandleLoop(HBasicBlock* loop_header) {
         loop_information->GetPreHeader()->GetSuccessors().size()));
   }
 
-  if (!GetGraph()->SuspendChecksAreAllowedToBeRemoved() &&
-      loop_information->GetSuspendCheck() == nullptr) {
-    AddError(StringPrintf("Loop with header %d does not have a suspend check.",
-                          loop_header->GetBlockId()));
+  if (loop_information->GetSuspendCheck() == nullptr) {
+    AddError(StringPrintf(
+        "Loop with header %d does not have a suspend check.",
+        loop_header->GetBlockId()));
   }
 
-  if (!GetGraph()->SuspendChecksAreAllowedToBeRemoved() &&
-      loop_information->GetSuspendCheck() != loop_header->GetFirstInstructionDisregardMoves()) {
+  if (loop_information->GetSuspendCheck() != loop_header->GetFirstInstructionDisregardMoves()) {
     AddError(StringPrintf(
         "Loop header %d does not have the loop suspend check as the first instruction.",
         loop_header->GetBlockId()));
diff --git a/compiler/optimizing/loop_optimization.cc b/compiler/optimizing/loop_optimization.cc
index cd054822cd..02ee4ec057 100644
--- a/compiler/optimizing/loop_optimization.cc
+++ b/compiler/optimizing/loop_optimization.cc
@@ -636,47 +636,6 @@ bool HLoopOptimization::TraverseLoopsInnerToOuter(LoopNode* node) {
 }
 
 //
-// This optimization applies to loops with plain simple operations
-// (I.e. no calls to java code or runtime) with a known small trip_count * instr_count
-// value.
-//
-bool HLoopOptimization::TryToRemoveSuspendCheckFromLoopHeader(LoopAnalysisInfo* analysis_info,
-                                                              bool generate_code) {
-  if (!graph_->SuspendChecksAreAllowedToBeRemoved()) {
-    return false;
-  }
-
-  int64_t trip_count = analysis_info->GetTripCount();
-
-  if (trip_count == LoopAnalysisInfo::kUnknownTripCount) {
-    return false;
-  }
-
-  int64_t instruction_count = analysis_info->GetNumberOfInstructions();
-  int64_t total_instruction_count = trip_count * instruction_count;
-
-  // The inclusion of the HasInstructionsPreventingScalarOpts() prevents this
-  // optimization from being applied to loops that have calls.
-  bool can_optimize =
-      total_instruction_count <= HLoopOptimization::kMaxTotalInstRemoveSuspendCheck &&
-      !analysis_info->HasInstructionsPreventingScalarOpts();
-
-  if (!can_optimize) {
-    return false;
-  }
-
-  if (generate_code) {
-    HLoopInformation* loop_info = analysis_info->GetLoopInfo();
-    HBasicBlock* header = loop_info->GetHeader();
-    HInstruction* instruction = header->GetLoopInformation()->GetSuspendCheck();
-    header->RemoveInstruction(instruction);
-    loop_info->SetSuspendCheck(nullptr);
-  }
-
-  return true;
-}
-
-//
 // Optimization.
 //
 
@@ -820,7 +779,7 @@ bool HLoopOptimization::TryOptimizeInnerLoopFinite(LoopNode* node) {
 }
 
 bool HLoopOptimization::OptimizeInnerLoop(LoopNode* node) {
-  return TryOptimizeInnerLoopFinite(node) || TryLoopScalarOpts(node);
+  return TryOptimizeInnerLoopFinite(node) || TryPeelingAndUnrolling(node);
 }
 
 
@@ -926,7 +885,7 @@ bool HLoopOptimization::TryFullUnrolling(LoopAnalysisInfo* analysis_info, bool g
   return true;
 }
 
-bool HLoopOptimization::TryLoopScalarOpts(LoopNode* node) {
+bool HLoopOptimization::TryPeelingAndUnrolling(LoopNode* node) {
   HLoopInformation* loop_info = node->loop_info;
   int64_t trip_count = LoopAnalysis::GetLoopTripCount(loop_info, &induction_range_);
   LoopAnalysisInfo analysis_info(loop_info);
@@ -939,16 +898,10 @@ bool HLoopOptimization::TryLoopScalarOpts(LoopNode* node) {
 
   if (!TryFullUnrolling(&analysis_info, /*generate_code*/ false) &&
       !TryPeelingForLoopInvariantExitsElimination(&analysis_info, /*generate_code*/ false) &&
-      !TryUnrollingForBranchPenaltyReduction(&analysis_info, /*generate_code*/ false) &&
-      !TryToRemoveSuspendCheckFromLoopHeader(&analysis_info, /*generate_code*/ false)) {
+      !TryUnrollingForBranchPenaltyReduction(&analysis_info, /*generate_code*/ false)) {
     return false;
   }
 
-  // Try the suspend check removal even for non-clonable loops. Also this
-  // optimization doesn't interfere with other scalar loop optimizations so it can
-  // be done prior to them.
-  bool removed_suspend_check = TryToRemoveSuspendCheckFromLoopHeader(&analysis_info);
-
   // Run 'IsLoopClonable' the last as it might be time-consuming.
   if (!LoopClonerHelper::IsLoopClonable(loop_info)) {
     return false;
@@ -956,7 +909,7 @@ bool HLoopOptimization::TryLoopScalarOpts(LoopNode* node) {
 
   return TryFullUnrolling(&analysis_info) ||
          TryPeelingForLoopInvariantExitsElimination(&analysis_info) ||
-         TryUnrollingForBranchPenaltyReduction(&analysis_info) || removed_suspend_check;
+         TryUnrollingForBranchPenaltyReduction(&analysis_info);
 }
 
 //
diff --git a/compiler/optimizing/loop_optimization.h b/compiler/optimizing/loop_optimization.h
index 0ca16b543d..d3583ed8a6 100644
--- a/compiler/optimizing/loop_optimization.h
+++ b/compiler/optimizing/loop_optimization.h
@@ -47,11 +47,6 @@ class HLoopOptimization : public HOptimization {
 
   static constexpr const char* kLoopOptimizationPassName = "loop_optimization";
 
-  // The maximum number of total instructions (trip_count * instruction_count),
-  // where the optimization of removing SuspendChecks from the loop header could
-  // be performed.
-  static constexpr int64_t kMaxTotalInstRemoveSuspendCheck = 128;
-
  private:
   /**
    * A single loop inside the loop hierarchy representation.
@@ -168,19 +163,8 @@ class HLoopOptimization : public HOptimization {
   // should be actually applied.
   bool TryFullUnrolling(LoopAnalysisInfo* analysis_info, bool generate_code = true);
 
-  // Tries to remove SuspendCheck for plain loops with a low trip count. The
-  // SuspendCheck in the codegen makes sure that the thread can be interrupted
-  // during execution for GC. Not being able to do so might decrease the
-  // responsiveness of GC when a very long loop or a long recursion is being
-  // executed. However, for plain loops with a small trip count, the removal of
-  // SuspendCheck should not affect the GC's responsiveness by a large margin.
-  // Consequently, since the thread won't be interrupted for plain loops, it is
-  // assumed that the performance might increase by removing SuspendCheck.
-  bool TryToRemoveSuspendCheckFromLoopHeader(LoopAnalysisInfo* analysis_info,
-                                             bool generate_code = true);
-
-  // Tries to apply scalar loop optimizations.
-  bool TryLoopScalarOpts(LoopNode* node);
+  // Tries to apply scalar loop peeling and unrolling.
+  bool TryPeelingAndUnrolling(LoopNode* node);
 
   //
   // Vectorization analysis and synthesis.
diff --git a/compiler/optimizing/nodes.h b/compiler/optimizing/nodes.h
index 94eae4d937..06fb88e837 100644
--- a/compiler/optimizing/nodes.h
+++ b/compiler/optimizing/nodes.h
@@ -675,13 +675,6 @@ class HGraph : public ArenaObject<kArenaAllocGraph> {
     return cha_single_implementation_list_;
   }
 
-  // In case of OSR we intend to use SuspendChecks as an entry point to the
-  // function; for debuggable graphs we might deoptimize to interpreter from
-  // SuspendChecks. In these cases we shouldn't remove them.
-  bool SuspendChecksAreAllowedToBeRemoved() const {
-    return !IsDebuggable() && !IsCompilingOsr();
-  }
-
   void AddCHASingleImplementationDependency(ArtMethod* method) {
     cha_single_implementation_list_.insert(method);
   }