Next bunch of fixes for check elimination.

- Canonicalize HCheckMapValue with constant map to
  HCheckMaps, and get rid of the special treatment
  during check elimination.
- Track only stable object maps for HConstants and
  add CHECK()s to verify state during code generation.

R=svenpanne@chromium.org

Review URL: https://codereview.chromium.org/263923004

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

diff --git a/src/arm/lithium-codegen-arm.cc b/src/arm/lithium-codegen-arm.cc
index 19b34da..6f73c1d 100644 (file)
--- a/src/arm/lithium-codegen-arm.cc
+++ b/src/arm/lithium-codegen-arm.cc
@@ -1873,9 +1873,15 @@ void LCodeGen::DoConstantE(LConstantE* instr) {
 
 
 void LCodeGen::DoConstantT(LConstantT* instr) {
-  Handle<Object> value = instr->value(isolate());
+  Handle<Object> object = instr->value(isolate());
   AllowDeferredHandleDereference smi_check;
-  __ Move(ToRegister(instr->result()), value);
+  if (instr->hydrogen()->HasObjectMap()) {
+    Handle<Map> object_map = instr->hydrogen()->ObjectMap().handle();
+    CHECK(object->IsHeapObject());
+    CHECK(!object_map->is_stable() ||
+          *object_map == Handle<HeapObject>::cast(object)->map());
+  }
+  __ Move(ToRegister(instr->result()), object);
 }
 
 

diff --git a/src/arm64/lithium-codegen-arm64.cc b/src/arm64/lithium-codegen-arm64.cc
index 17243c8..fd9dd5d 100644 (file)
--- a/src/arm64/lithium-codegen-arm64.cc
+++ b/src/arm64/lithium-codegen-arm64.cc
@@ -2531,9 +2531,15 @@ void LCodeGen::DoConstantS(LConstantS* instr) {
 
 
 void LCodeGen::DoConstantT(LConstantT* instr) {
-  Handle<Object> value = instr->value(isolate());
+  Handle<Object> object = instr->value(isolate());
   AllowDeferredHandleDereference smi_check;
-  __ LoadObject(ToRegister(instr->result()), value);
+  if (instr->hydrogen()->HasObjectMap()) {
+    Handle<Map> object_map = instr->hydrogen()->ObjectMap().handle();
+    CHECK(object->IsHeapObject());
+    CHECK(!object_map->is_stable() ||
+          *object_map == Handle<HeapObject>::cast(object)->map());
+  }
+  __ LoadObject(ToRegister(instr->result()), object);
 }
 
 

diff --git a/src/hydrogen-check-elimination.cc b/src/hydrogen-check-elimination.cc
index fb7812e..bf549c5 100644 (file)
--- a/src/hydrogen-check-elimination.cc
+++ b/src/hydrogen-check-elimination.cc
@@ -70,10 +70,6 @@ class HCheckTable : public ZoneObject {
             HTransitionElementsKind::cast(instr));
         break;
       }
-      case HValue::kCheckMapValue: {
-        ReduceCheckMapValue(HCheckMapValue::cast(instr));
-        break;
-      }
       case HValue::kCheckHeapObject: {
         ReduceCheckHeapObject(HCheckHeapObject::cast(instr));
         break;
@@ -361,39 +357,6 @@ class HCheckTable : public ZoneObject {
     INC_STAT(loads_);
   }
 
-  void ReduceCheckMapValue(HCheckMapValue* instr) {
-    if (!instr->map()->IsConstant()) return;  // Nothing to learn.
-
-    HValue* object = instr->value()->ActualValue();
-    // Match a HCheckMapValue(object, HConstant(map))
-    Unique<Map> map = MapConstant(instr->map());
-
-    HCheckTableEntry* entry = Find(object);
-    if (entry != NULL) {
-      if (entry->maps_->Contains(map)) {
-        if (entry->maps_->size() == 1) {
-          // Object is known to have exactly this map.
-          if (entry->check_ != NULL) {
-            instr->DeleteAndReplaceWith(entry->check_);
-          } else {
-            // Mark check as dead but leave it in the graph as a checkpoint for
-            // subsequent checks.
-            instr->SetFlag(HValue::kIsDead);
-            entry->check_ = instr;
-          }
-          INC_STAT(removed_);
-        } else {
-          // Only one map survives the check.
-          entry->maps_ = new(zone()) UniqueSet<Map>(map, zone());
-          entry->check_ = instr;
-        }
-      }
-    } else {
-      // No prior information.
-      Insert(object, instr, map);
-    }
-  }
-
   void ReduceCheckHeapObject(HCheckHeapObject* instr) {
     if (FindMaps(instr->value()->ActualValue()) != NULL) {
       // If the object has known maps, it's definitely a heap object.
@@ -407,12 +370,12 @@ class HCheckTable : public ZoneObject {
     if (instr->has_transition()) {
       // This store transitions the object to a new map.
       Kill(object);
-      Insert(object, NULL, MapConstant(instr->transition()));
+      Insert(object, NULL, HConstant::cast(instr->transition())->MapValue());
     } else if (instr->access().IsMap()) {
       // This is a store directly to the map field of the object.
       Kill(object);
       if (!instr->value()->IsConstant()) return;
-      Insert(object, NULL, MapConstant(instr->value()));
+      Insert(object, NULL, HConstant::cast(instr->value())->MapValue());
     } else {
       // If the instruction changes maps, it should be handled above.
       CHECK(!instr->CheckChangesFlag(kMaps));
@@ -587,10 +550,6 @@ class HCheckTable : public ZoneObject {
     if (size_ < kMaxTrackedObjects) size_++;
   }
 
-  Unique<Map> MapConstant(HValue* value) {
-    return Unique<Map>::cast(HConstant::cast(value)->GetUnique());
-  }
-
   Zone* zone() const { return phase_->zone(); }
 
   friend class HCheckMapsEffects;

diff --git a/src/hydrogen-instructions.cc b/src/hydrogen-instructions.cc
index c1a23f9..d13b7ee 100644 (file)
--- a/src/hydrogen-instructions.cc
+++ b/src/hydrogen-instructions.cc
@@ -1380,6 +1380,17 @@ void HCheckMapValue::PrintDataTo(StringStream* stream) {
 }
 
 
+HValue* HCheckMapValue::Canonicalize() {
+  if (map()->IsConstant()) {
+    HConstant* c_map = HConstant::cast(map());
+    return HCheckMaps::CreateAndInsertAfter(
+        block()->graph()->zone(), value(), c_map->MapValue(),
+        c_map->HasStableMapValue(), this);
+  }
+  return this;
+}
+
+
 void HForInPrepareMap::PrintDataTo(StringStream* stream) {
   enumerable()->PrintNameTo(stream);
 }
@@ -1682,7 +1693,7 @@ void HCheckMaps::PrintDataTo(StringStream* stream) {
 HValue* HCheckMaps::Canonicalize() {
   if (!IsStabilityCheck() && maps_are_stable() && value()->IsConstant()) {
     HConstant* c_value = HConstant::cast(value());
-    if (c_value->HasObjectMap() && c_value->ObjectMapIsStable()) {
+    if (c_value->HasObjectMap()) {
       for (int i = 0; i < maps()->size(); ++i) {
         if (c_value->ObjectMap() == maps()->at(i)) {
           if (maps()->size() > 1) {
@@ -2685,7 +2696,7 @@ HConstant::HConstant(Handle<Object> object, Representation r)
   : HTemplateInstruction<0>(HType::TypeFromValue(object)),
     object_(Unique<Object>::CreateUninitialized(object)),
     object_map_(Handle<Map>::null()),
-    object_map_is_stable_(false),
+    has_stable_map_value_(false),
     has_smi_value_(false),
     has_int32_value_(false),
     has_double_value_(false),
@@ -2695,13 +2706,15 @@ HConstant::HConstant(Handle<Object> object, Representation r)
     is_undetectable_(false),
     instance_type_(kUnknownInstanceType) {
   if (object->IsHeapObject()) {
-    Isolate* isolate = Handle<HeapObject>::cast(object)->GetIsolate();
-    Handle<Map> map(Handle<HeapObject>::cast(object)->map(), isolate);
+    Handle<HeapObject> heap_object = Handle<HeapObject>::cast(object);
+    Isolate* isolate = heap_object->GetIsolate();
+    Handle<Map> map(heap_object->map(), isolate);
     is_not_in_new_space_ = !isolate->heap()->InNewSpace(*object);
     instance_type_ = map->instance_type();
     is_undetectable_ = map->is_undetectable();
-    object_map_ = Unique<Map>::CreateImmovable(map);
-    object_map_is_stable_ = map->is_stable();
+    if (map->is_stable()) object_map_ = Unique<Map>::CreateImmovable(map);
+    has_stable_map_value_ = (instance_type_ == MAP_TYPE &&
+                             Handle<Map>::cast(heap_object)->is_stable());
   }
   if (object->IsNumber()) {
     double n = object->Number();
@@ -2717,7 +2730,9 @@ HConstant::HConstant(Handle<Object> object, Representation r)
 }
 
 
-HConstant::HConstant(Unique<Object> unique,
+HConstant::HConstant(Unique<Object> object,
+                     Unique<Map> object_map,
+                     bool has_stable_map_value,
                      Representation r,
                      HType type,
                      bool is_not_in_new_space,
@@ -2725,9 +2740,9 @@ HConstant::HConstant(Unique<Object> unique,
                      bool is_undetectable,
                      InstanceType instance_type)
   : HTemplateInstruction<0>(type),
-    object_(unique),
-    object_map_(Handle<Map>::null()),
-    object_map_is_stable_(false),
+    object_(object),
+    object_map_(object_map),
+    has_stable_map_value_(has_stable_map_value),
     has_smi_value_(false),
     has_int32_value_(false),
     has_double_value_(false),
@@ -2736,7 +2751,7 @@ HConstant::HConstant(Unique<Object> unique,
     boolean_value_(boolean_value),
     is_undetectable_(is_undetectable),
     instance_type_(instance_type) {
-  ASSERT(!unique.handle().is_null());
+  ASSERT(!object.handle().is_null());
   ASSERT(!type.IsTaggedNumber());
   Initialize(r);
 }
@@ -2748,7 +2763,7 @@ HConstant::HConstant(int32_t integer_value,
                      Unique<Object> object)
   : object_(object),
     object_map_(Handle<Map>::null()),
-    object_map_is_stable_(false),
+    has_stable_map_value_(false),
     has_smi_value_(Smi::IsValid(integer_value)),
     has_int32_value_(true),
     has_double_value_(true),
@@ -2774,7 +2789,7 @@ HConstant::HConstant(double double_value,
                      Unique<Object> object)
   : object_(object),
     object_map_(Handle<Map>::null()),
-    object_map_is_stable_(false),
+    has_stable_map_value_(false),
     has_int32_value_(IsInteger32(double_value)),
     has_double_value_(true),
     has_external_reference_value_(false),
@@ -2798,7 +2813,7 @@ HConstant::HConstant(ExternalReference reference)
   : HTemplateInstruction<0>(HType::None()),
     object_(Unique<Object>(Handle<Object>::null())),
     object_map_(Handle<Map>::null()),
-    object_map_is_stable_(false),
+    has_stable_map_value_(false),
     has_smi_value_(false),
     has_int32_value_(false),
     has_double_value_(false),
@@ -2904,6 +2919,8 @@ HConstant* HConstant::CopyToRepresentation(Representation r, Zone* zone) const {
   }
   ASSERT(!object_.handle().is_null());
   return new(zone) HConstant(object_,
+                             object_map_,
+                             has_stable_map_value_,
                              r,
                              type_,
                              is_not_in_new_space_,
@@ -2955,6 +2972,13 @@ void HConstant::PrintDataTo(StringStream* stream) {
             external_reference_value_.address()));
   } else {
     handle(Isolate::Current())->ShortPrint(stream);
+    stream->Add(" ");
+    if (HasStableMapValue()) {
+      stream->Add("[stable-map] ");
+    }
+    if (HasObjectMap()) {
+      stream->Add("[map %p] ", *ObjectMap().handle());
+    }
   }
   if (!is_not_in_new_space_) {
     stream->Add("[new space] ");

diff --git a/src/hydrogen-instructions.h b/src/hydrogen-instructions.h
index 1219b83..0ec127e 100644 (file)
--- a/src/hydrogen-instructions.h
+++ b/src/hydrogen-instructions.h
@@ -2785,6 +2785,23 @@ class HCheckMaps V8_FINAL : public HTemplateInstruction<2> {
 
   virtual HValue* Canonicalize() V8_OVERRIDE;
 
+  static HCheckMaps* CreateAndInsertAfter(Zone* zone,
+                                          HValue* value,
+                                          Unique<Map> map,
+                                          bool map_is_stable,
+                                          HInstruction* instr) {
+    return CreateAndInsertAfter(zone, value, new(zone) UniqueSet<Map>(
+            map, zone), map_is_stable, instr);
+  }
+
+  static HCheckMaps* CreateAndInsertAfter(Zone* zone,
+                                          HValue* value,
+                                          const UniqueSet<Map>* maps,
+                                          bool maps_are_stable,
+                                          HInstruction* instr) {
+    return instr->Append(new(zone) HCheckMaps(value, maps, maps_are_stable));
+  }
+
   DECLARE_CONCRETE_INSTRUCTION(CheckMaps)
 
  protected:
@@ -2795,7 +2812,20 @@ class HCheckMaps V8_FINAL : public HTemplateInstruction<2> {
   virtual int RedefinedOperandIndex() { return 0; }
 
  private:
-  // Clients should use one of the static New* methods above.
+  HCheckMaps(HValue* value, const UniqueSet<Map>* maps, bool maps_are_stable)
+      : HTemplateInstruction<2>(value->type()), maps_(maps),
+        has_migration_target_(false), is_stability_check_(false),
+        maps_are_stable_(maps_are_stable) {
+    ASSERT_NE(0, maps->size());
+    SetOperandAt(0, value);
+    // Use the object value for the dependency.
+    SetOperandAt(1, value);
+    set_representation(Representation::Tagged());
+    SetFlag(kUseGVN);
+    SetDependsOnFlag(kMaps);
+    SetDependsOnFlag(kElementsKind);
+  }
+
   HCheckMaps(HValue* value, const UniqueSet<Map>* maps, HValue* typecheck)
       : HTemplateInstruction<2>(value->type()), maps_(maps),
         has_migration_target_(false), is_stability_check_(false),
@@ -3463,20 +3493,22 @@ class HConstant V8_FINAL : public HTemplateInstruction<0> {
   }
 
   static HConstant* CreateAndInsertBefore(Zone* zone,
-                                          Unique<Object> unique,
+                                          Unique<Object> object,
                                           bool is_not_in_new_space,
                                           HInstruction* instruction) {
     return instruction->Prepend(new(zone) HConstant(
-        unique, Representation::Tagged(), HType::Tagged(),
-        is_not_in_new_space, false, false, kUnknownInstanceType));
+        object, Unique<Map>(Handle<Map>::null()), false,
+        Representation::Tagged(), HType::Tagged(), is_not_in_new_space,
+        false, false, kUnknownInstanceType));
   }
 
   static HConstant* CreateAndInsertAfter(Zone* zone,
-                                         Unique<Map> unique,
+                                         Unique<Map> map,
                                          HInstruction* instruction) {
     return instruction->Append(new(zone) HConstant(
-            unique, Representation::Tagged(), HType::Tagged(),
-            true, false, false, MAP_TYPE));
+            map, Unique<Map>(Handle<Map>::null()), false,
+            Representation::Tagged(), HType::Tagged(), true,
+            false, false, MAP_TYPE));
   }
 
   Handle<Object> handle(Isolate* isolate) {
@@ -3575,15 +3607,21 @@ class HConstant V8_FINAL : public HTemplateInstruction<0> {
   bool IsUndetectable() const { return is_undetectable_; }
   InstanceType GetInstanceType() const { return instance_type_; }
 
+  bool HasMapValue() const { return instance_type_ == MAP_TYPE; }
+  Unique<Map> MapValue() const {
+    ASSERT(HasMapValue());
+    return Unique<Map>::cast(GetUnique());
+  }
+  bool HasStableMapValue() const {
+    ASSERT(HasMapValue());
+    return has_stable_map_value_;
+  }
+
   bool HasObjectMap() const { return !object_map_.IsNull(); }
   Unique<Map> ObjectMap() const {
     ASSERT(HasObjectMap());
     return object_map_;
   }
-  bool ObjectMapIsStable() const {
-    ASSERT(HasObjectMap());
-    return object_map_is_stable_;
-  }
 
   virtual intptr_t Hashcode() V8_OVERRIDE {
     if (has_int32_value_) {
@@ -3657,7 +3695,9 @@ class HConstant V8_FINAL : public HTemplateInstruction<0> {
             Representation r = Representation::None(),
             bool is_not_in_new_space = true,
             Unique<Object> optional = Unique<Object>(Handle<Object>::null()));
-  HConstant(Unique<Object> unique,
+  HConstant(Unique<Object> object,
+            Unique<Map> object_map,
+            bool has_stable_map_value,
             Representation r,
             HType type,
             bool is_not_in_new_space,
@@ -3677,9 +3717,11 @@ class HConstant V8_FINAL : public HTemplateInstruction<0> {
   // constant HeapObject.
   Unique<Object> object_;
 
-  // If this is a heap object, this points to the Map of the object.
+  // If object_ is a heap object, this points to the stable map of the object.
   Unique<Map> object_map_;
-  bool object_map_is_stable_ : 1;
+
+  // If object_ is a map, this indicates whether the map is stable.
+  bool has_stable_map_value_ : 1;
 
   // We store the HConstant in the most specific form safely possible.
   // The two flags, has_int32_value_ and has_double_value_ tell us if
@@ -7461,6 +7503,8 @@ class HCheckMapValue V8_FINAL : public HTemplateInstruction<2> {
   HValue* value() { return OperandAt(0); }
   HValue* map() { return OperandAt(1); }
 
+  virtual HValue* Canonicalize() V8_OVERRIDE;
+
   DECLARE_CONCRETE_INSTRUCTION(CheckMapValue)
 
  protected:

diff --git a/src/hydrogen.cc b/src/hydrogen.cc
index 679c7d4..787428d 100644 (file)
--- a/src/hydrogen.cc
+++ b/src/hydrogen.cc
@@ -678,11 +678,13 @@ HConstant* HGraph::GetConstantMinus1() {
 }
 
 
-#define DEFINE_GET_CONSTANT(Name, name, htype, boolean_value)                  \
+#define DEFINE_GET_CONSTANT(Name, name, type, htype, boolean_value)            \
 HConstant* HGraph::GetConstant##Name() {                                       \
   if (!constant_##name##_.is_set()) {                                          \
     HConstant* constant = new(zone()) HConstant(                               \
         Unique<Object>::CreateImmovable(isolate()->factory()->name##_value()), \
+        Unique<Map>::CreateImmovable(isolate()->factory()->type##_map()),      \
+        false,                                                                 \
         Representation::Tagged(),                                              \
         htype,                                                                 \
         true,                                                                  \
@@ -696,11 +698,11 @@ HConstant* HGraph::GetConstant##Name() {                                       \
 }
 
 
-DEFINE_GET_CONSTANT(Undefined, undefined, HType::Tagged(), false)
-DEFINE_GET_CONSTANT(True, true, HType::Boolean(), true)
-DEFINE_GET_CONSTANT(False, false, HType::Boolean(), false)
-DEFINE_GET_CONSTANT(Hole, the_hole, HType::Tagged(), false)
-DEFINE_GET_CONSTANT(Null, null, HType::Tagged(), false)
+DEFINE_GET_CONSTANT(Undefined, undefined, undefined, HType::Tagged(), false)
+DEFINE_GET_CONSTANT(True, true, boolean, HType::Boolean(), true)
+DEFINE_GET_CONSTANT(False, false, boolean, HType::Boolean(), false)
+DEFINE_GET_CONSTANT(Hole, the_hole, the_hole, HType::Tagged(), false)
+DEFINE_GET_CONSTANT(Null, null, null, HType::Tagged(), false)
 
 
 #undef DEFINE_GET_CONSTANT

diff --git a/src/ia32/lithium-codegen-ia32.cc b/src/ia32/lithium-codegen-ia32.cc
index ba43e42..ee37d0b 100644 (file)
--- a/src/ia32/lithium-codegen-ia32.cc
+++ b/src/ia32/lithium-codegen-ia32.cc
@@ -2011,9 +2011,15 @@ void LCodeGen::DoConstantE(LConstantE* instr) {
 
 void LCodeGen::DoConstantT(LConstantT* instr) {
   Register reg = ToRegister(instr->result());
-  Handle<Object> handle = instr->value(isolate());
+  Handle<Object> object = instr->value(isolate());
   AllowDeferredHandleDereference smi_check;
-  __ LoadObject(reg, handle);
+  if (instr->hydrogen()->HasObjectMap()) {
+    Handle<Map> object_map = instr->hydrogen()->ObjectMap().handle();
+    CHECK(object->IsHeapObject());
+    CHECK(!object_map->is_stable() ||
+          *object_map == Handle<HeapObject>::cast(object)->map());
+  }
+  __ LoadObject(reg, object);
 }
 
 

diff --git a/src/mips/lithium-codegen-mips.cc b/src/mips/lithium-codegen-mips.cc
index e8593d9..bdbc010 100644 (file)
--- a/src/mips/lithium-codegen-mips.cc
+++ b/src/mips/lithium-codegen-mips.cc
@@ -1728,9 +1728,15 @@ void LCodeGen::DoConstantE(LConstantE* instr) {
 
 
 void LCodeGen::DoConstantT(LConstantT* instr) {
-  Handle<Object> value = instr->value(isolate());
+  Handle<Object> object = instr->value(isolate());
   AllowDeferredHandleDereference smi_check;
-  __ li(ToRegister(instr->result()), value);
+  if (instr->hydrogen()->HasObjectMap()) {
+    Handle<Map> object_map = instr->hydrogen()->ObjectMap().handle();
+    CHECK(object->IsHeapObject());
+    CHECK(!object_map->is_stable() ||
+          *object_map == Handle<HeapObject>::cast(object)->map());
+  }
+  __ li(ToRegister(instr->result()), object);
 }
 
 

diff --git a/src/x64/lithium-codegen-x64.cc b/src/x64/lithium-codegen-x64.cc
index c49fa93..0cb42dc 100644 (file)
--- a/src/x64/lithium-codegen-x64.cc
+++ b/src/x64/lithium-codegen-x64.cc
@@ -1691,8 +1691,14 @@ void LCodeGen::DoConstantE(LConstantE* instr) {
 
 
 void LCodeGen::DoConstantT(LConstantT* instr) {
-  Handle<Object> value = instr->value(isolate());
-  __ Move(ToRegister(instr->result()), value);
+  Handle<Object> object = instr->value(isolate());
+  if (instr->hydrogen()->HasObjectMap()) {
+    Handle<Map> object_map = instr->hydrogen()->ObjectMap().handle();
+    CHECK(object->IsHeapObject());
+    CHECK(!object_map->is_stable() ||
+          *object_map == Handle<HeapObject>::cast(object)->map());
+  }
+  __ Move(ToRegister(instr->result()), object);
 }