Make the array bounds check elimination phase optional (and set the foundation for introducing SSI in a simple way).

BUG=

Review URL: https://chromiumcodereview.appspot.com/11783055

git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@13409 ce2b1a6d-e550-0410-aec6-3dcde31c8c00

src/arm/lithium-codegen-arm.cc

@@ -494,8 +494,6 @@ bool LCodeGen::IsInteger32(LConstantOperand* op) const {
 
 int LCodeGen::ToInteger32(LConstantOperand* op) const {
   HConstant* constant = chunk_->LookupConstant(op);
-  ASSERT(chunk_->LookupLiteralRepresentation(op).IsInteger32());
-  ASSERT(constant->HasInteger32Value());
   return constant->Integer32Value();
 }
 

src/code-stubs-hydrogen.cc

@@ -40,7 +40,7 @@ Handle<Code> HydrogenCodeStub::CodeFromGraph(HGraph* graph) {
   graph->AssignDominators();
   graph->CollectPhis();
   graph->InsertRepresentationChanges();
-  graph->EliminateRedundantBoundsChecks();
+  graph->ApplyActualValues();
   LChunk* chunk = LChunk::NewChunk(graph);
   ASSERT(chunk != NULL);
   Handle<Code> stub = chunk->Codegen(Code::COMPILED_STUB);
@@ -123,7 +123,8 @@ void CodeStubGraphBuilder<KeyedLoadFastElementStub>::BuildCodeStub() {
 
   HInstruction* load = BuildUncheckedMonomorphicElementAccess(
       GetParameter(0), GetParameter(1), NULL, NULL,
-      casted_stub()->is_js_array(), casted_stub()->elements_kind(), false);
+      casted_stub()->is_js_array(), casted_stub()->elements_kind(),
+      false, Representation::Tagged());
   AddInstruction(load);
 
   HReturn* ret = new(zone) HReturn(load, context());

src/hydrogen-instructions.cc

@@ -760,6 +760,28 @@ void HBoundsCheck::PrintDataTo(StringStream* stream) {
 }
 
 
+void HBoundsCheck::InferRepresentation(HInferRepresentation* h_infer) {
+  ASSERT(CheckFlag(kFlexibleRepresentation));
+  Representation r;
+  if (key_mode_ == DONT_ALLOW_SMI_KEY ||
+      !length()->representation().IsTagged()) {
+    r = Representation::Integer32();
+  } else if (index()->representation().IsTagged() ||
+      (index()->IsConstant() &&
+       HConstant::cast(index())->HasInteger32Value() &&
+       Smi::IsValid(HConstant::cast(index())->Integer32Value()))) {
+    // If the index is tagged, or a constant that holds a Smi, allow the length
+    // to be tagged, since it is usually already tagged from loading it out of
+    // the length field of a JSArray. This allows for direct comparison without
+    // untagging.
+    r = Representation::Tagged();
+  } else {
+    r = Representation::Integer32();
+  }
+  UpdateRepresentation(r, h_infer, "boundscheck");
+}
+
+
 void HCallConstantFunction::PrintDataTo(StringStream* stream) {
   if (IsApplyFunction()) {
     stream->Add("optimized apply ");

src/hydrogen-instructions.h

@@ -672,6 +672,14 @@ class HValue: public ZoneObject {
     return NULL;
   }
 
+  // There are HInstructions that do not really change a value, they
+  // only add pieces of information to it (like bounds checks, map checks
+  // or SSI definitions after conditional branches).
+  // This method must always return the original HValue SSA definition (the
+  // register allocator relies on this to avoid allocating multiple registers
+  // for the same value).
+  virtual HValue* ActualValue() { return this; }
+
   bool IsDefinedAfter(HBasicBlock* other) const;
 
   // Operands.
@@ -2957,44 +2965,40 @@ enum BoundsCheckKeyMode {
 class HBoundsCheck: public HTemplateInstruction<2> {
  public:
   HBoundsCheck(HValue* index, HValue* length,
-               BoundsCheckKeyMode key_mode = DONT_ALLOW_SMI_KEY)
+               BoundsCheckKeyMode key_mode = DONT_ALLOW_SMI_KEY,
+               Representation r = Representation::None())
       : key_mode_(key_mode) {
     SetOperandAt(0, index);
     SetOperandAt(1, length);
-    set_representation(Representation::Integer32());
+    if (r.IsNone()) {
+      // In the normal compilation pipeline the representation is flexible
+      // (see comment to RequiredInputRepresentation).
+      SetFlag(kFlexibleRepresentation);
+    } else {
+      // When compiling stubs we want to set the representation explicitly
+      // so the compilation pipeline can skip the HInferRepresentation phase.
+      set_representation(r);
+    }
     SetFlag(kUseGVN);
   }
 
   virtual Representation RequiredInputRepresentation(int arg_index) {
-    if (key_mode_ == DONT_ALLOW_SMI_KEY ||
+    return representation();
-        !length()->representation().IsTagged()) {
-      return Representation::Integer32();
-    }
-    // If the index is tagged and isn't constant, then allow the length
-    // to be tagged, since it is usually already tagged from loading it out of
-    // the length field of a JSArray. This allows for direct comparison without
-    // untagging.
-    if (index()->representation().IsTagged() && !index()->IsConstant()) {
-      return Representation::Tagged();
-    }
-    // Also allow the length to be tagged if the index is constant, because
-    // it can be tagged to allow direct comparison.
-    if (index()->IsConstant() &&
-        index()->representation().IsInteger32() &&
-        arg_index == 1) {
-      return Representation::Tagged();
-    }
-    return Representation::Integer32();
   }
   virtual Representation observed_input_representation(int index) {
     return Representation::Integer32();
   }
 
   virtual void PrintDataTo(StringStream* stream);
+  virtual void InferRepresentation(HInferRepresentation* h_infer);
 
   HValue* index() { return OperandAt(0); }
   HValue* length() { return OperandAt(1); }
 
+  virtual HValue* ActualValue() {
+    return index();
+  }
+
   DECLARE_CONCRETE_INSTRUCTION(BoundsCheck)
 
  protected:
@@ -4365,6 +4369,11 @@ class ArrayInstructionInterface {
   virtual bool IsDehoisted() = 0;
   virtual void SetDehoisted(bool is_dehoisted) = 0;
   virtual ~ArrayInstructionInterface() { };
+
+  static Representation KeyedAccessIndexRequirement(Representation r) {
+    return r.IsInteger32()
+        ? Representation::Integer32() : Representation::Tagged();
+  }
 };
 
 
@@ -4448,7 +4457,10 @@ class HLoadKeyed
       return is_external() ? Representation::External()
           : Representation::Tagged();
     }
-    if (index == 1) return Representation::Integer32();
+    if (index == 1) {
+      return ArrayInstructionInterface::KeyedAccessIndexRequirement(
+          OperandAt(1)->representation());
+    }
     return Representation::None();
   }
 
@@ -4663,7 +4675,8 @@ class HStoreKeyed
       return is_external() ? Representation::External()
                            : Representation::Tagged();
     } else if (index == 1) {
-      return Representation::Integer32();
+      return ArrayInstructionInterface::KeyedAccessIndexRequirement(
+          OperandAt(1)->representation());
     }
 
     ASSERT_EQ(index, 2);

src/hydrogen.cc

@@ -743,7 +743,8 @@ HInstruction* HGraphBuilder::BuildUncheckedMonomorphicElementAccess(
     HCheckMaps* mapcheck,
     bool is_js_array,
     ElementsKind elements_kind,
-    bool is_store) {
+    bool is_store,
+    Representation checked_index_representation) {
   Zone* zone = this->zone();
   // No GVNFlag is necessary for ElementsKind if there is an explicit dependency
   // on a HElementsTransition instruction. The flag can also be removed if the
@@ -771,8 +772,8 @@ HInstruction* HGraphBuilder::BuildUncheckedMonomorphicElementAccess(
   HInstruction* checked_key = NULL;
   if (IsExternalArrayElementsKind(elements_kind)) {
     length = AddInstruction(new(zone) HFixedArrayBaseLength(elements));
-    checked_key = AddInstruction(new(zone) HBoundsCheck(key, length,
+    checked_key = AddInstruction(new(zone) HBoundsCheck(
-                                                        ALLOW_SMI_KEY));
+        key, length, ALLOW_SMI_KEY, checked_index_representation));
     HLoadExternalArrayPointer* external_elements =
         new(zone) HLoadExternalArrayPointer(elements);
     AddInstruction(external_elements);
@@ -789,8 +790,8 @@ HInstruction* HGraphBuilder::BuildUncheckedMonomorphicElementAccess(
   } else {
     length = AddInstruction(new(zone) HFixedArrayBaseLength(elements));
   }
-  checked_key = AddInstruction(new(zone) HBoundsCheck(key, length,
+  checked_key = AddInstruction(new(zone) HBoundsCheck(
-                                                      ALLOW_SMI_KEY));
+      key, length, ALLOW_SMI_KEY, checked_index_representation));
   return BuildFastElementAccess(elements, checked_key, val, mapcheck,
                                 elements_kind, is_store);
 }
@@ -3503,10 +3504,12 @@ bool HGraph::Optimize(SmartArrayPointer<char>* bailout_reason) {
   HStackCheckEliminator sce(this);
   sce.Process();
 
-  EliminateRedundantBoundsChecks();
+  if (FLAG_array_bounds_checks_elimination) EliminateRedundantBoundsChecks();
-  DehoistSimpleArrayIndexComputations();
+  if (FLAG_array_index_dehoisting) DehoistSimpleArrayIndexComputations();
   if (FLAG_dead_code_elimination) DeadCodeElimination();
 
+  ApplyActualValues();
+
   return true;
 }
 
@@ -3666,7 +3669,7 @@ class BoundsCheckBbData: public ZoneObject {
     }
 
     if (!keep_new_check) {
-      new_check->DeleteAndReplaceWith(NULL);
+      new_check->DeleteAndReplaceWith(new_check->ActualValue());
     }
 
     return true;
@@ -3802,10 +3805,6 @@ void HGraph::EliminateRedundantBoundsChecks(HBasicBlock* bb,
     if (!i->IsBoundsCheck()) continue;
 
     HBoundsCheck* check = HBoundsCheck::cast(i);
-    check->ReplaceAllUsesWith(check->index());
-
-    if (!FLAG_array_bounds_checks_elimination) continue;
-
     int32_t offset;
     BoundsCheckKey* key =
         BoundsCheckKey::Create(zone(), check, &offset);
@@ -3823,7 +3822,7 @@ void HGraph::EliminateRedundantBoundsChecks(HBasicBlock* bb,
                                                    NULL);
       *data_p = bb_data_list;
     } else if (data->OffsetIsCovered(offset)) {
-      check->DeleteAndReplaceWith(NULL);
+      check->DeleteAndReplaceWith(check->ActualValue());
     } else if (data->BasicBlock() != bb ||
                !data->CoverCheck(check, offset)) {
       // If the check is in the current BB we try to modify it by calling
@@ -3920,8 +3919,6 @@ static void DehoistArrayIndex(ArrayInstructionInterface* array_operation) {
 
 
 void HGraph::DehoistSimpleArrayIndexComputations() {
-  if (!FLAG_array_index_dehoisting) return;
-
   HPhase phase("H_Dehoist index computations", this);
   for (int i = 0; i < blocks()->length(); ++i) {
     for (HInstruction* instr = blocks()->at(i)->first();
@@ -3973,6 +3970,30 @@ void HGraph::DeadCodeElimination() {
 }
 
 
+void HGraph::ApplyActualValues() {
+  HPhase phase("H_Apply actual values", this);
+
+  for (int block_index = 0; block_index < blocks()->length(); block_index++) {
+    HBasicBlock* block = blocks()->at(block_index);
+
+#ifdef DEBUG
+    for (int i = 0; i < block->phis()->length(); i++) {
+      HPhi* phi = block->phis()->at(i);
+      ASSERT(phi->ActualValue() == phi);
+    }
+#endif
+
+    for (HInstruction* instruction = block->first();
+        instruction != NULL;
+        instruction = instruction->next()) {
+      if (instruction->ActualValue() != instruction) {
+        instruction->ReplaceAllUsesWith(instruction->ActualValue());
+      }
+    }
+  }
+}
+
+
 void HOptimizedGraphBuilder::AddPhi(HPhi* instr) {
   ASSERT(current_block() != NULL);
   current_block()->AddPhi(instr);

src/hydrogen.h

@@ -273,6 +273,7 @@ class HGraph: public ZoneObject {
   void EliminateRedundantBoundsChecks();
   void DehoistSimpleArrayIndexComputations();
   void DeadCodeElimination();
+  void ApplyActualValues();
   void PropagateDeoptimizingMark();
   void EliminateUnusedInstructions();
 
@@ -879,7 +880,8 @@ class HGraphBuilder {
       HCheckMaps* mapcheck,
       bool is_js_array,
       ElementsKind elements_kind,
-      bool is_store);
+      bool is_store,
+      Representation checked_index_representation = Representation::None());
 
  private:
   HGraphBuilder();

src/ia32/lithium-codegen-ia32.cc

@@ -495,8 +495,6 @@ XMMRegister LCodeGen::ToDoubleRegister(LOperand* op) const {
 
 int LCodeGen::ToInteger32(LConstantOperand* op) const {
   HConstant* constant = chunk_->LookupConstant(op);
-  ASSERT(chunk_->LookupLiteralRepresentation(op).IsInteger32());
-  ASSERT(constant->HasInteger32Value());
   return constant->Integer32Value();
 }
 
@@ -3200,13 +3198,6 @@ Operand LCodeGen::BuildFastArrayOperand(
     uint32_t additional_index) {
   Register elements_pointer_reg = ToRegister(elements_pointer);
   int shift_size = ElementsKindToShiftSize(elements_kind);
-  // Even though the HLoad/StoreKeyed instructions force the input
-  // representation for the key to be an integer, the input gets replaced during
-  // bound check elimination with the index argument to the bounds check, which
-  // can be tagged, so that case must be handled here, too.
-  if (key_representation.IsTagged() && (shift_size >= 1)) {
-    shift_size -= kSmiTagSize;
-  }
   if (key->IsConstantOperand()) {
     int constant_value = ToInteger32(LConstantOperand::cast(key));
     if (constant_value & 0xF0000000) {
@@ -3216,6 +3207,10 @@ Operand LCodeGen::BuildFastArrayOperand(
                    ((constant_value + additional_index) << shift_size)
                        + offset);
   } else {
+    // Take the tag bit into account while computing the shift size.
+    if (key_representation.IsTagged() && (shift_size >= 1)) {
+      shift_size -= kSmiTagSize;
+    }
     ScaleFactor scale_factor = static_cast<ScaleFactor>(shift_size);
     return Operand(elements_pointer_reg,
                    ToRegister(key),

src/mips/lithium-codegen-mips.cc

@@ -481,8 +481,6 @@ bool LCodeGen::IsInteger32(LConstantOperand* op) const {
 
 int LCodeGen::ToInteger32(LConstantOperand* op) const {
   HConstant* constant = chunk_->LookupConstant(op);
-  ASSERT(chunk_->LookupLiteralRepresentation(op).IsInteger32());
-  ASSERT(constant->HasInteger32Value());
   return constant->Integer32Value();
 }
 

src/x64/lithium-codegen-x64.cc

@@ -416,8 +416,6 @@ bool LCodeGen::IsTaggedConstant(LConstantOperand* op) const {
 
 int LCodeGen::ToInteger32(LConstantOperand* op) const {
   HConstant* constant = chunk_->LookupConstant(op);
-  ASSERT(chunk_->LookupLiteralRepresentation(op).IsInteger32());
-  ASSERT(constant->HasInteger32Value());
   return constant->Integer32Value();
 }