ART: Formatting and comments in BooleanSimplifier

Change-Id: I9a5aa3f2aa8b0a29d7b0f1e5e247397cf8e9e379

9 files changed, 65 insertions, 42 deletions

diff --git a/compiler/optimizing/boolean_simplifier.cc b/compiler/optimizing/boolean_simplifier.cc
index ecf9fa2..0ecc0d7 100644
--- a/compiler/optimizing/boolean_simplifier.cc
+++ b/compiler/optimizing/boolean_simplifier.cc
@@ -18,15 +18,6 @@
 
 namespace art {
 
-static bool EndsWithAnIf(HBasicBlock* block) {
-  return block->GetLastInstruction()->IsIf();
-}
-
-static bool HasSinglePhi(HBasicBlock* block) {
-  return !block->GetPhis().IsEmpty()
-      && block->GetFirstPhi()->GetNext() == nullptr;
-}
-
 // Returns true if 'block1' and 'block2' are empty, merge into the same single
 // successor and the successor can only be reached from them.
 static bool BlocksDoMergeTogether(HBasicBlock* block1, HBasicBlock* block2) {
@@ -39,15 +30,15 @@ static bool BlocksDoMergeTogether(HBasicBlock* block1, HBasicBlock* block2) {
 // Returns true if the outcome of the branching matches the boolean value of
 // the branching condition.
 static bool PreservesCondition(HInstruction* input_true, HInstruction* input_false) {
-  return input_true->IsIntConstant() && input_true->AsIntConstant()->GetValue() == 1
-         && input_false->IsIntConstant() && input_false->AsIntConstant()->GetValue() == 0;
+  return input_true->IsIntConstant() && input_true->AsIntConstant()->IsOne()
+      && input_false->IsIntConstant() && input_false->AsIntConstant()->IsZero();
 }
 
 // Returns true if the outcome of the branching is exactly opposite of the
 // boolean value of the branching condition.
 static bool NegatesCondition(HInstruction* input_true, HInstruction* input_false) {
-  return input_true->IsIntConstant() && input_true->AsIntConstant()->GetValue() == 0
-         && input_false->IsIntConstant() && input_false->AsIntConstant()->GetValue() == 1;
+  return input_true->IsIntConstant() && input_true->AsIntConstant()->IsZero()
+      && input_false->IsIntConstant() && input_false->AsIntConstant()->IsOne();
 }
 
 // Returns an instruction with the opposite boolean value from 'cond'.
@@ -72,11 +63,11 @@ static HInstruction* GetOppositeCondition(HInstruction* cond) {
       return new (allocator) HLessThan(lhs, rhs);
     }
   } else if (cond->IsIntConstant()) {
-    int32_t value = cond->AsIntConstant()->GetValue();
-    if (value == 0) {
+    HIntConstant* int_const = cond->AsIntConstant();
+    if (int_const->IsZero()) {
       return graph->GetIntConstant1();
     } else {
-      DCHECK_EQ(value, 1);
+      DCHECK(int_const->IsOne());
       return graph->GetIntConstant0();
     }
   }
@@ -91,7 +82,7 @@ void HBooleanSimplifier::Run() {
   // order does not matter.
   for (HPostOrderIterator it(*graph_); !it.Done(); it.Advance()) {
     HBasicBlock* block = it.Current();
-    if (!EndsWithAnIf(block)) continue;
+    if (!block->EndsWithIf()) continue;
 
     // Find elements of the pattern.
     HIf* if_instruction = block->GetLastInstruction()->AsIf();
@@ -101,7 +92,7 @@ void HBooleanSimplifier::Run() {
       continue;
     }
     HBasicBlock* merge_block = true_block->GetSuccessors().Get(0);
-    if (!HasSinglePhi(merge_block)) {
+    if (!merge_block->HasSinglePhi()) {
       continue;
     }
     HPhi* phi = merge_block->GetFirstPhi()->AsPhi();
diff --git a/compiler/optimizing/boolean_simplifier.h b/compiler/optimizing/boolean_simplifier.h
index 9fa9c5a..a88733e 100644
--- a/compiler/optimizing/boolean_simplifier.h
+++ b/compiler/optimizing/boolean_simplifier.h
@@ -15,7 +15,7 @@
  */
 
 // This optimization recognizes a common pattern where a boolean value is
-// either casted to an integer or negated by selecting from zero/one integer
+// either cast to an integer or negated by selecting from zero/one integer
 // constants with an If statement. Because boolean values are internally
 // represented as zero/one, we can safely replace the pattern with a suitable
 // condition instruction.
diff --git a/compiler/optimizing/code_generator_arm.cc b/compiler/optimizing/code_generator_arm.cc
index d783903..5a79a69 100644
--- a/compiler/optimizing/code_generator_arm.cc
+++ b/compiler/optimizing/code_generator_arm.cc
@@ -1388,8 +1388,8 @@ void LocationsBuilderARM::VisitTypeConversion(HTypeConversion* conversion) {
   LocationSummary* locations =
       new (GetGraph()->GetArena()) LocationSummary(conversion, call_kind);
 
-  // Java language does not allow treating boolean as an integral type but our
-  // bit representation makes it safe.
+  // The Java language does not allow treating boolean as an integral type but
+  // our bit representation makes it safe.
 
   switch (result_type) {
     case Primitive::kPrimByte:
diff --git a/compiler/optimizing/code_generator_x86.cc b/compiler/optimizing/code_generator_x86.cc
index 0a7d3fe..4414a65 100644
--- a/compiler/optimizing/code_generator_x86.cc
+++ b/compiler/optimizing/code_generator_x86.cc
@@ -1370,8 +1370,8 @@ void LocationsBuilderX86::VisitTypeConversion(HTypeConversion* conversion) {
   LocationSummary* locations =
       new (GetGraph()->GetArena()) LocationSummary(conversion, call_kind);
 
-  // Java language does not allow treating boolean as an integral type but our
-  // bit representation makes it safe.
+  // The Java language does not allow treating boolean as an integral type but
+  // our bit representation makes it safe.
 
   switch (result_type) {
     case Primitive::kPrimByte:
diff --git a/compiler/optimizing/code_generator_x86_64.cc b/compiler/optimizing/code_generator_x86_64.cc
index bff8fc9..c1f601e 100644
--- a/compiler/optimizing/code_generator_x86_64.cc
+++ b/compiler/optimizing/code_generator_x86_64.cc
@@ -1410,8 +1410,8 @@ void LocationsBuilderX86_64::VisitTypeConversion(HTypeConversion* conversion) {
   Primitive::Type input_type = conversion->GetInputType();
   DCHECK_NE(result_type, input_type);
 
-  // Java language does not allow treating boolean as an integral type but our
-  // bit representation makes it safe.
+  // The Java language does not allow treating boolean as an integral type but
+  // our bit representation makes it safe.
 
   switch (result_type) {
     case Primitive::kPrimByte:
diff --git a/compiler/optimizing/nodes.cc b/compiler/optimizing/nodes.cc
index 6009cb5..4f6565d 100644
--- a/compiler/optimizing/nodes.cc
+++ b/compiler/optimizing/nodes.cc
@@ -832,6 +832,14 @@ bool HBasicBlock::IsSingleGoto() const {
          && (loop_info == nullptr || !loop_info->IsBackEdge(*this));
 }
 
+bool HBasicBlock::EndsWithIf() const {
+  return !GetInstructions().IsEmpty() && GetLastInstruction()->IsIf();
+}
+
+bool HBasicBlock::HasSinglePhi() const {
+  return !GetPhis().IsEmpty() && GetFirstPhi()->GetNext() == nullptr;
+}
+
 void HInstructionList::SetBlockOfInstructions(HBasicBlock* block) const {
   for (HInstruction* current = first_instruction_;
        current != nullptr;
@@ -1086,13 +1094,15 @@ void HGraph::InlineInto(HGraph* outer_graph, HInvoke* invoke) {
 }
 
 void HGraph::MergeEmptyBranches(HBasicBlock* start_block, HBasicBlock* end_block) {
-  // Make sure this is a diamond control-flow path, find the two branches.
+  // Find the two branches of an If.
   DCHECK_EQ(start_block->GetSuccessors().Size(), 2u);
-  DCHECK_EQ(end_block->GetPredecessors().Size(), 2u);
   HBasicBlock* left_branch = start_block->GetSuccessors().Get(0);
   HBasicBlock* right_branch = start_block->GetSuccessors().Get(1);
+
+  // Make sure this is a diamond control-flow path.
   DCHECK_EQ(left_branch->GetSuccessors().Get(0), end_block);
   DCHECK_EQ(right_branch->GetSuccessors().Get(0), end_block);
+  DCHECK_EQ(end_block->GetPredecessors().Size(), 2u);
   DCHECK_EQ(start_block, end_block->GetDominator());
 
   // Disconnect the branches and merge the two blocks. This will move
@@ -1114,16 +1124,12 @@ void HGraph::MergeEmptyBranches(HBasicBlock* start_block, HBasicBlock* end_block
   reverse_post_order_.Delete(right_branch);
   reverse_post_order_.Delete(end_block);
 
-  // Update loop information.
-  HLoopInformation* loop_info = start_block->GetLoopInformation();
-  if (kIsDebugBuild) {
-    if (loop_info != nullptr) {
-      DCHECK_EQ(loop_info, left_branch->GetLoopInformation());
-      DCHECK_EQ(loop_info, right_branch->GetLoopInformation());
-      DCHECK_EQ(loop_info, end_block->GetLoopInformation());
-    }
-  }
-  while (loop_info != nullptr) {
+  // Update loops which contain the code.
+  for (HLoopInformationOutwardIterator it(*start_block); !it.Done(); it.Advance()) {
+    HLoopInformation* loop_info = it.Current();
+    DCHECK(loop_info->Contains(*left_branch));
+    DCHECK(loop_info->Contains(*right_branch));
+    DCHECK(loop_info->Contains(*end_block));
     loop_info->Remove(left_branch);
     loop_info->Remove(right_branch);
     loop_info->Remove(end_block);
@@ -1131,8 +1137,6 @@ void HGraph::MergeEmptyBranches(HBasicBlock* start_block, HBasicBlock* end_block
       loop_info->RemoveBackEdge(end_block);
       loop_info->AddBackEdge(start_block);
     }
-    // Move to parent loop if nested.
-    loop_info = loop_info->GetHeader()->GetDominator()->GetLoopInformation();
   }
 }
 
diff --git a/compiler/optimizing/nodes.h b/compiler/optimizing/nodes.h
index db7873b..664cf18 100644
--- a/compiler/optimizing/nodes.h
+++ b/compiler/optimizing/nodes.h
@@ -602,6 +602,9 @@ class HBasicBlock : public ArenaObject<kArenaAllocMisc> {
   bool IsCatchBlock() const { return is_catch_block_; }
   void SetIsCatchBlock() { is_catch_block_ = true; }
 
+  bool EndsWithIf() const;
+  bool HasSinglePhi() const;
+
  private:
   HGraph* graph_;
   GrowableArray<HBasicBlock*> predecessors_;
@@ -624,6 +627,31 @@ class HBasicBlock : public ArenaObject<kArenaAllocMisc> {
   DISALLOW_COPY_AND_ASSIGN(HBasicBlock);
 };
 
+// Iterates over the LoopInformation of all loops which contain 'block'
+// from the innermost to the outermost.
+class HLoopInformationOutwardIterator : public ValueObject {
+ public:
+  explicit HLoopInformationOutwardIterator(const HBasicBlock& block)
+      : current_(block.GetLoopInformation()) {}
+
+  bool Done() const { return current_ == nullptr; }
+
+  void Advance() {
+    DCHECK(!Done());
+    current_ = current_->GetHeader()->GetDominator()->GetLoopInformation();
+  }
+
+  HLoopInformation* Current() const {
+    DCHECK(!Done());
+    return current_;
+  }
+
+ private:
+  HLoopInformation* current_;
+
+  DISALLOW_COPY_AND_ASSIGN(HLoopInformationOutwardIterator);
+};
+
 #define FOR_EACH_CONCRETE_INSTRUCTION(M)                                \
   M(Add, BinaryOperation)                                               \
   M(And, BinaryOperation)                                               \
diff --git a/runtime/primitive.h b/runtime/primitive.h
index d11f1e9..32bfdaf 100644
--- a/runtime/primitive.h
+++ b/runtime/primitive.h
@@ -153,8 +153,8 @@ class Primitive {
   }
 
   static bool IsIntegralType(Type type) {
-    // Java language does not allow treating boolean as an integral type but our
-    // bit representation makes it safe.
+    // The Java language does not allow treating boolean as an integral type but
+    // our bit representation makes it safe.
     switch (type) {
       case kPrimBoolean:
       case kPrimByte:
diff --git a/test/463-checker-boolean-simplifier/src/Main.java b/test/463-checker-boolean-simplifier/src/Main.java
index 25f58b4..efe0d3f 100644
--- a/test/463-checker-boolean-simplifier/src/Main.java
+++ b/test/463-checker-boolean-simplifier/src/Main.java
@@ -116,7 +116,7 @@ public class Main {
   // CHECK-DAG:                       Return [ [[Cond]] ]
 
   public static boolean LessThan(int x, int y) {
-    return x < y;
+    return (x < y) ? true : false;
   }
 
   /*