Revert r13409 ("Make the array bounds check elimination phase optional (and...

Revert r13409 ("Make the array bounds check elimination phase optional (and set the foundation for introducing SSI in a simple way).") because of a crash in octane/mandreel.

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

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

src/arm/lithium-codegen-arm.cc

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

src/hydrogen-instructions.cc

@@ -765,28 +765,6 @@ 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

@@ -673,14 +673,6 @@ 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.
@@ -2988,40 +2980,44 @@ enum BoundsCheckKeyMode {
 class HBoundsCheck: public HTemplateInstruction<2> {
  public:
   HBoundsCheck(HValue* index, HValue* length,
-               BoundsCheckKeyMode key_mode = DONT_ALLOW_SMI_KEY,
-               Representation r = Representation::None())
+               BoundsCheckKeyMode key_mode = DONT_ALLOW_SMI_KEY)
       : key_mode_(key_mode) {
     SetOperandAt(0, index);
     SetOperandAt(1, length);
-    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);
-    }
+    set_representation(Representation::Integer32());
     SetFlag(kUseGVN);
   }
 
   virtual Representation RequiredInputRepresentation(int arg_index) {
-    return representation();
+    if (key_mode_ == DONT_ALLOW_SMI_KEY ||
+        !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:
@@ -4392,11 +4388,6 @@ 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();
-  }
 };
 
 
@@ -4480,10 +4471,7 @@ class HLoadKeyed
       return is_external() ? Representation::External()
           : Representation::Tagged();
     }
-    if (index == 1) {
-      return ArrayInstructionInterface::KeyedAccessIndexRequirement(
-          OperandAt(1)->representation());
-    }
+    if (index == 1) return Representation::Integer32();
     return Representation::None();
   }
 
@@ -4698,8 +4686,7 @@ class HStoreKeyed
       return is_external() ? Representation::External()
                            : Representation::Tagged();
     } else if (index == 1) {
-      return ArrayInstructionInterface::KeyedAccessIndexRequirement(
-          OperandAt(1)->representation());
+      return Representation::Integer32();
     }
 
     ASSERT_EQ(index, 2);

src/hydrogen.cc

@@ -743,8 +743,7 @@ HInstruction* HGraphBuilder::BuildUncheckedMonomorphicElementAccess(
     HCheckMaps* mapcheck,
     bool is_js_array,
     ElementsKind elements_kind,
-    bool is_store,
-    Representation checked_index_representation) {
+    bool is_store) {
   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
@@ -772,8 +771,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, ALLOW_SMI_KEY, checked_index_representation));
+    checked_key = AddInstruction(new(zone) HBoundsCheck(key, length,
+                                                        ALLOW_SMI_KEY));
     HLoadExternalArrayPointer* external_elements =
         new(zone) HLoadExternalArrayPointer(elements);
     AddInstruction(external_elements);
@@ -790,8 +789,8 @@ HInstruction* HGraphBuilder::BuildUncheckedMonomorphicElementAccess(
   } else {
     length = AddInstruction(new(zone) HFixedArrayBaseLength(elements));
   }
-  checked_key = AddInstruction(new(zone) HBoundsCheck(
-      key, length, ALLOW_SMI_KEY, checked_index_representation));
+  checked_key = AddInstruction(new(zone) HBoundsCheck(key, length,
+                                                      ALLOW_SMI_KEY));
   return BuildFastElementAccess(elements, checked_key, val, mapcheck,
                                 elements_kind, is_store);
 }
@@ -3565,12 +3564,10 @@ bool HGraph::Optimize(SmartArrayPointer<char>* bailout_reason) {
   HStackCheckEliminator sce(this);
   sce.Process();
 
-  if (FLAG_array_bounds_checks_elimination) EliminateRedundantBoundsChecks();
-  if (FLAG_array_index_dehoisting) DehoistSimpleArrayIndexComputations();
+  EliminateRedundantBoundsChecks();
+  DehoistSimpleArrayIndexComputations();
   if (FLAG_dead_code_elimination) DeadCodeElimination();
 
-  ApplyActualValues();
-
   return true;
 }
 
@@ -3730,7 +3727,7 @@ class BoundsCheckBbData: public ZoneObject {
     }
 
     if (!keep_new_check) {
-      new_check->DeleteAndReplaceWith(new_check->ActualValue());
+      new_check->DeleteAndReplaceWith(NULL);
     }
 
     return true;
@@ -3866,6 +3863,10 @@ 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);
@@ -3883,7 +3884,7 @@ void HGraph::EliminateRedundantBoundsChecks(HBasicBlock* bb,
                                                    NULL);
       *data_p = bb_data_list;
     } else if (data->OffsetIsCovered(offset)) {
-      check->DeleteAndReplaceWith(check->ActualValue());
+      check->DeleteAndReplaceWith(NULL);
     } else if (data->BasicBlock() != bb ||
                !data->CoverCheck(check, offset)) {
       // If the check is in the current BB we try to modify it by calling
@@ -3980,6 +3981,8 @@ 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();
@@ -4031,30 +4034,6 @@ 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,7 +273,6 @@ class HGraph: public ZoneObject {
   void EliminateRedundantBoundsChecks();
   void DehoistSimpleArrayIndexComputations();
   void DeadCodeElimination();
-  void ApplyActualValues();
   void PropagateDeoptimizingMark();
   void EliminateUnusedInstructions();
 
@@ -890,8 +889,7 @@ class HGraphBuilder {
       HCheckMaps* mapcheck,
       bool is_js_array,
       ElementsKind elements_kind,
-      bool is_store,
-      Representation checked_index_representation = Representation::None());
+      bool is_store);
 
  private:
   HGraphBuilder();

src/ia32/lithium-codegen-ia32.cc

@@ -495,6 +495,8 @@ 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();
 }
 
@@ -3198,6 +3200,13 @@ 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) {
@@ -3207,10 +3216,6 @@ 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,6 +481,8 @@ 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,6 +416,8 @@ 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();
 }