Use UniqueSet<T> and Unique<T> in HCheckMaps and HCheckValue.

BUG=
R=verwaest@chromium.org

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

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

src/arm/lithium-codegen-arm.cc

@@ -5150,7 +5150,7 @@ void LCodeGen::DoCheckInstanceType(LCheckInstanceType* instr) {
 
 void LCodeGen::DoCheckValue(LCheckValue* instr) {
   Register reg = ToRegister(instr->value());
-  Handle<HeapObject> object = instr->hydrogen()->object();
+  Handle<HeapObject> object = instr->hydrogen()->object().handle();
   AllowDeferredHandleDereference smi_check;
   if (isolate()->heap()->InNewSpace(*object)) {
     Register reg = ToRegister(instr->value());
@@ -5202,7 +5202,6 @@ void LCodeGen::DoCheckMaps(LCheckMaps* instr) {
   ASSERT(input->IsRegister());
   Register reg = ToRegister(input);
 
-  SmallMapList* map_set = instr->hydrogen()->map_set();
   __ ldr(map_reg, FieldMemOperand(reg, HeapObject::kMapOffset));
 
   DeferredCheckMaps* deferred = NULL;
@@ -5211,14 +5210,15 @@ void LCodeGen::DoCheckMaps(LCheckMaps* instr) {
     __ bind(deferred->check_maps());
   }
 
+  UniqueSet<Map> map_set = instr->hydrogen()->map_set();
   Label success;
-  for (int i = 0; i < map_set->length() - 1; i++) {
-    Handle<Map> map = map_set->at(i);
+  for (int i = 0; i < map_set.size() - 1; i++) {
+    Handle<Map> map = map_set.at(i).handle();
     __ CompareMap(map_reg, map, &success);
     __ b(eq, &success);
   }
 
-  Handle<Map> map = map_set->last();
+  Handle<Map> map = map_set.at(map_set.size() - 1).handle();
   __ CompareMap(map_reg, map, &success);
   if (instr->hydrogen()->has_migration_target()) {
     __ b(ne, deferred->entry());

src/hydrogen-escape-analysis.cc

@@ -154,9 +154,8 @@ HValue* HEscapeAnalysisPhase::NewMapCheckAndInsert(HCapturedObject* state,
   HValue* value = state->map_value();
   // TODO(mstarzinger): This will narrow a map check against a set of maps
   // down to the first element in the set. Revisit and fix this.
-  Handle<Map> map_object = mapcheck->map_set()->first();
-  UniqueValueId map_id = mapcheck->map_unique_ids()->first();
-  HCheckValue* check = HCheckValue::New(zone, NULL, value, map_object, map_id);
+  HCheckValue* check = HCheckValue::New(
+      zone, NULL, value, mapcheck->first_map(), false);
   check->InsertBefore(mapcheck);
   return check;
 }

src/hydrogen-instructions.cc

@@ -1431,11 +1431,9 @@ void HCheckMaps::HandleSideEffectDominator(GVNFlag side_effect,
     HStoreNamedField* store = HStoreNamedField::cast(dominator);
     if (!store->has_transition() || store->object() != value()) return;
     HConstant* transition = HConstant::cast(store->transition());
-    for (int i = 0; i < map_set()->length(); i++) {
-      if (transition->UniqueValueIdsMatch(map_unique_ids_.at(i))) {
-        DeleteAndReplaceWith(NULL);
-        return;
-      }
+    if (map_set_.Contains(transition->GetUnique())) {
+      DeleteAndReplaceWith(NULL);
+      return;
     }
   }
 }
@@ -1443,9 +1441,9 @@ void HCheckMaps::HandleSideEffectDominator(GVNFlag side_effect,
 
 void HCheckMaps::PrintDataTo(StringStream* stream) {
   value()->PrintNameTo(stream);
-  stream->Add(" [%p", *map_set()->first());
-  for (int i = 1; i < map_set()->length(); ++i) {
-    stream->Add(",%p", *map_set()->at(i));
+  stream->Add(" [%p", *map_set_.at(0).handle());
+  for (int i = 1; i < map_set_.size(); ++i) {
+    stream->Add(",%p", *map_set_.at(i).handle());
   }
   stream->Add("]%s", CanOmitMapChecks() ? "(omitted)" : "");
 }
@@ -1454,13 +1452,13 @@ void HCheckMaps::PrintDataTo(StringStream* stream) {
 void HCheckValue::PrintDataTo(StringStream* stream) {
   value()->PrintNameTo(stream);
   stream->Add(" ");
-  object()->ShortPrint(stream);
+  object().handle()->ShortPrint(stream);
 }
 
 
 HValue* HCheckValue::Canonicalize() {
   return (value()->IsConstant() &&
-          HConstant::cast(value())->UniqueValueIdsMatch(object_unique_id_))
+          HConstant::cast(value())->GetUnique() == object_)
       ? NULL
       : this;
 }
@@ -2929,22 +2927,17 @@ HCheckMaps* HCheckMaps::New(Zone* zone,
   if (map->CanOmitMapChecks() &&
       value->IsConstant() &&
       HConstant::cast(value)->HasMap(map)) {
-    check_map->omit(info);
+    // TODO(titzer): collect dependent map checks into a list.
+    check_map->omit_ = true;
+    if (map->CanTransition()) {
+      map->AddDependentCompilationInfo(
+          DependentCode::kPrototypeCheckGroup, info);
+    }
   }
   return check_map;
 }
 
 
-void HCheckMaps::FinalizeUniqueValueId() {
-  if (!map_unique_ids_.is_empty()) return;
-  Zone* zone = block()->zone();
-  map_unique_ids_.Initialize(map_set_.length(), zone);
-  for (int i = 0; i < map_set_.length(); i++) {
-    map_unique_ids_.Add(UniqueValueId(map_set_.at(i)), zone);
-  }
-}
-
-
 void HLoadNamedGeneric::PrintDataTo(StringStream* stream) {
   object()->PrintNameTo(stream);
   stream->Add(".");

src/hydrogen-instructions.h

@@ -36,6 +36,7 @@
 #include "deoptimizer.h"
 #include "small-pointer-list.h"
 #include "string-stream.h"
+#include "unique.h"
 #include "v8conversions.h"
 #include "v8utils.h"
 #include "zone.h"
@@ -2603,7 +2604,6 @@ class HCheckMaps V8_FINAL : public HTemplateInstruction<2> {
     for (int i = 0; i < maps->length(); i++) {
       check_map->Add(maps->at(i), zone);
     }
-    check_map->map_set_.Sort();
     return check_map;
   }
 
@@ -2618,38 +2618,26 @@ class HCheckMaps V8_FINAL : public HTemplateInstruction<2> {
   virtual void PrintDataTo(StringStream* stream) V8_OVERRIDE;
 
   HValue* value() { return OperandAt(0); }
-  SmallMapList* map_set() { return &map_set_; }
-  ZoneList<UniqueValueId>* map_unique_ids() { return &map_unique_ids_; }
 
-  bool has_migration_target() {
+  Unique<Map> first_map() const { return map_set_.at(0); }
+  UniqueSet<Map> map_set() const { return map_set_; }
+
+  bool has_migration_target() const {
     return has_migration_target_;
   }
 
-  virtual void FinalizeUniqueValueId() V8_OVERRIDE;
-
   DECLARE_CONCRETE_INSTRUCTION(CheckMaps)
 
  protected:
   virtual bool DataEquals(HValue* other) V8_OVERRIDE {
-    ASSERT_EQ(map_set_.length(), map_unique_ids_.length());
-    HCheckMaps* b = HCheckMaps::cast(other);
-    // Relies on the fact that map_set has been sorted before.
-    if (map_unique_ids_.length() != b->map_unique_ids_.length()) {
-      return false;
-    }
-    for (int i = 0; i < map_unique_ids_.length(); i++) {
-      if (map_unique_ids_.at(i) != b->map_unique_ids_.at(i)) {
-        return false;
-      }
-    }
-    return true;
+    return this->map_set_.Equals(&HCheckMaps::cast(other)->map_set_);
   }
 
   virtual int RedefinedOperandIndex() { return 0; }
 
  private:
   void Add(Handle<Map> map, Zone* zone) {
-    map_set_.Add(map, zone);
+    map_set_.Add(Unique<Map>(map), zone);
     if (!has_migration_target_ && map->is_migration_target()) {
       has_migration_target_ = true;
       SetGVNFlag(kChangesNewSpacePromotion);
@@ -2659,10 +2647,9 @@ class HCheckMaps V8_FINAL : public HTemplateInstruction<2> {
   // Clients should use one of the static New* methods above.
   HCheckMaps(HValue* value, Zone *zone, HValue* typecheck)
       : HTemplateInstruction<2>(value->type()),
-        omit_(false), has_migration_target_(false), map_unique_ids_(0, zone) {
+        omit_(false), has_migration_target_(false) {
     SetOperandAt(0, value);
     // Use the object value for the dependency if NULL is passed.
-    // TODO(titzer): do GVN flags already express this dependency?
     SetOperandAt(1, typecheck != NULL ? typecheck : value);
     set_representation(Representation::Tagged());
     SetFlag(kUseGVN);
@@ -2671,36 +2658,33 @@ class HCheckMaps V8_FINAL : public HTemplateInstruction<2> {
     SetGVNFlag(kDependsOnElementsKind);
   }
 
-  void omit(CompilationInfo* info) {
-    omit_ = true;
-    for (int i = 0; i < map_set_.length(); i++) {
-      Handle<Map> map = map_set_.at(i);
-      if (!map->CanTransition()) continue;
-      map->AddDependentCompilationInfo(DependentCode::kPrototypeCheckGroup,
-                                       info);
-    }
-  }
-
   bool omit_;
   bool has_migration_target_;
-  SmallMapList map_set_;
-  ZoneList<UniqueValueId> map_unique_ids_;
+  UniqueSet<Map> map_set_;
 };
 
 
 class HCheckValue V8_FINAL : public HUnaryOperation {
  public:
   static HCheckValue* New(Zone* zone, HValue* context,
-                          HValue* value, Handle<JSFunction> target) {
-    bool in_new_space = zone->isolate()->heap()->InNewSpace(*target);
+                          HValue* value, Handle<JSFunction> func) {
+    bool in_new_space = zone->isolate()->heap()->InNewSpace(*func);
+    // NOTE: We create an uninitialized Unique and initialize it later.
+    // This is because a JSFunction can move due to GC during graph creation.
+    // TODO(titzer): This is a migration crutch. Replace with some kind of
+    // Uniqueness scope later.
+    Unique<JSFunction> target = Unique<JSFunction>::CreateUninitialized(func);
     HCheckValue* check = new(zone) HCheckValue(value, target, in_new_space);
     return check;
   }
   static HCheckValue* New(Zone* zone, HValue* context,
-                          HValue* value, Handle<Map> map, UniqueValueId id) {
-    HCheckValue* check = new(zone) HCheckValue(value, map, false);
-    check->object_unique_id_ = id;
-    return check;
+                          HValue* value, Unique<HeapObject> target,
+                          bool object_in_new_space) {
+    return new(zone) HCheckValue(value, target, object_in_new_space);
+  }
+
+  virtual void FinalizeUniqueValueId() V8_OVERRIDE {
+    object_ = Unique<HeapObject>(object_.handle());
   }
 
   virtual Representation RequiredInputRepresentation(int index) V8_OVERRIDE {
@@ -2714,11 +2698,7 @@ class HCheckValue V8_FINAL : public HUnaryOperation {
   virtual void Verify() V8_OVERRIDE;
 #endif
 
-  virtual void FinalizeUniqueValueId() V8_OVERRIDE {
-    object_unique_id_ = UniqueValueId(object_);
-  }
-
-  Handle<HeapObject> object() const { return object_; }
+  Unique<HeapObject> object() const { return object_; }
   bool object_in_new_space() const { return object_in_new_space_; }
 
   DECLARE_CONCRETE_INSTRUCTION(CheckValue)
@@ -2726,19 +2706,20 @@ class HCheckValue V8_FINAL : public HUnaryOperation {
  protected:
   virtual bool DataEquals(HValue* other) V8_OVERRIDE {
     HCheckValue* b = HCheckValue::cast(other);
-    return object_unique_id_ == b->object_unique_id_;
+    return object_ == b->object_;
   }
 
  private:
-  HCheckValue(HValue* value, Handle<HeapObject> object, bool in_new_space)
+  HCheckValue(HValue* value, Unique<HeapObject> object,
+               bool object_in_new_space)
       : HUnaryOperation(value, value->type()),
-        object_(object), object_in_new_space_(in_new_space) {
+        object_(object),
+        object_in_new_space_(object_in_new_space) {
     set_representation(Representation::Tagged());
     SetFlag(kUseGVN);
   }
 
-  Handle<HeapObject> object_;
-  UniqueValueId object_unique_id_;
+  Unique<HeapObject> object_;
   bool object_in_new_space_;
 };
 
@@ -3486,6 +3467,12 @@ class HConstant V8_FINAL : public HTemplateInstruction<0> {
         unique_id_ == other;
   }
 
+  Unique<Object> GetUnique() const {
+    // TODO(titzer): store a Unique<HeapObject> inside the HConstant.
+    Address raw_address = reinterpret_cast<Address>(unique_id_.Hashcode());
+    return Unique<Object>(raw_address, handle_);
+  }
+
 #ifdef DEBUG
   virtual void Verify() V8_OVERRIDE { }
 #endif

src/ia32/lithium-codegen-ia32.cc

@@ -5622,7 +5622,7 @@ void LCodeGen::DoCheckInstanceType(LCheckInstanceType* instr) {
 
 
 void LCodeGen::DoCheckValue(LCheckValue* instr) {
-  Handle<HeapObject> object = instr->hydrogen()->object();
+  Handle<HeapObject> object = instr->hydrogen()->object().handle();
   if (instr->hydrogen()->object_in_new_space()) {
     Register reg = ToRegister(instr->value());
     Handle<Cell> cell = isolate()->factory()->NewCell(object);
@@ -5677,22 +5677,21 @@ void LCodeGen::DoCheckMaps(LCheckMaps* instr) {
   ASSERT(input->IsRegister());
   Register reg = ToRegister(input);
 
-  SmallMapList* map_set = instr->hydrogen()->map_set();
-
   DeferredCheckMaps* deferred = NULL;
   if (instr->hydrogen()->has_migration_target()) {
     deferred = new(zone()) DeferredCheckMaps(this, instr, reg, x87_stack_);
     __ bind(deferred->check_maps());
   }
 
+  UniqueSet<Map> map_set = instr->hydrogen()->map_set();
   Label success;
-  for (int i = 0; i < map_set->length() - 1; i++) {
-    Handle<Map> map = map_set->at(i);
+  for (int i = 0; i < map_set.size() - 1; i++) {
+    Handle<Map> map = map_set.at(i).handle();
     __ CompareMap(reg, map, &success);
     __ j(equal, &success);
   }
 
-  Handle<Map> map = map_set->last();
+  Handle<Map> map = map_set.at(map_set.size() - 1).handle();
   __ CompareMap(reg, map, &success);
   if (instr->hydrogen()->has_migration_target()) {
     __ j(not_equal, deferred->entry());

src/mips/lithium-codegen-mips.cc

@@ -5092,7 +5092,7 @@ void LCodeGen::DoCheckInstanceType(LCheckInstanceType* instr) {
 
 void LCodeGen::DoCheckValue(LCheckValue* instr) {
   Register reg = ToRegister(instr->value());
-  Handle<HeapObject> object = instr->hydrogen()->object();
+  Handle<HeapObject> object = instr->hydrogen()->object().handle();
   AllowDeferredHandleDereference smi_check;
   if (isolate()->heap()->InNewSpace(*object)) {
     Register reg = ToRegister(instr->value());
@@ -5143,7 +5143,6 @@ void LCodeGen::DoCheckMaps(LCheckMaps* instr) {
   LOperand* input = instr->value();
   ASSERT(input->IsRegister());
   Register reg = ToRegister(input);
-  SmallMapList* map_set = instr->hydrogen()->map_set();
   __ lw(map_reg, FieldMemOperand(reg, HeapObject::kMapOffset));
 
   DeferredCheckMaps* deferred = NULL;
@@ -5152,12 +5151,13 @@ void LCodeGen::DoCheckMaps(LCheckMaps* instr) {
     __ bind(deferred->check_maps());
   }
 
+  UniqueSet<Map> map_set = instr->hydrogen()->map_set();
   Label success;
-  for (int i = 0; i < map_set->length() - 1; i++) {
-    Handle<Map> map = map_set->at(i);
+  for (int i = 0; i < map_set.size() - 1; i++) {
+    Handle<Map> map = map_set.at(i).handle();
     __ CompareMapAndBranch(map_reg, map, &success, eq, &success);
   }
-  Handle<Map> map = map_set->last();
+  Handle<Map> map = map_set.at(map_set.size() - 1).handle();
   // Do the CompareMap() directly within the Branch() and DeoptimizeIf().
   if (instr->hydrogen()->has_migration_target()) {
     __ Branch(deferred->entry(), ne, map_reg, Operand(map));

src/unique.h

@@ -83,29 +83,41 @@ class Unique V8_FINAL {
 
   template <typename U>
   bool operator==(const Unique<U>& other) const {
+    ASSERT(IsInitialized() && other.IsInitialized());
     return raw_address_ == other.raw_address_;
   }
 
   template <typename U>
   bool operator!=(const Unique<U>& other) const {
+    ASSERT(IsInitialized() && other.IsInitialized());
     return raw_address_ != other.raw_address_;
   }
 
   intptr_t Hashcode() const {
+    ASSERT(IsInitialized());
     return reinterpret_cast<intptr_t>(raw_address_);
   }
 
-  bool IsNull() {
+  bool IsNull() const {
+    ASSERT(IsInitialized());
     return raw_address_ == NULL;
   }
 
-  // Don't do this unless you have access to the heap!
-  // No, seriously! You can compare and hash and set-ify uniques that were
-  // all created at the same time; please don't dereference.
-  Handle<T> handle() {
+  // Extract the handle from this Unique in order to dereference it.
+  // WARNING: Only do this if you have access to the heap.
+  Handle<T> handle() const {
     return handle_;
   }
 
+  bool IsInitialized() const {
+    return raw_address_ != NULL || handle_.is_null();
+  }
+
+  // TODO(titzer): this is a hack to migrate to Unique<T> incrementally.
+  static Unique<T> CreateUninitialized(Handle<T> handle) {
+    return Unique<T>(static_cast<Address>(NULL), handle);
+  }
+
   friend class UniqueSet<T>;  // Uses internal details for speed.
   template <class U>
   friend class Unique;  // For comparing raw_address values.
@@ -124,6 +136,7 @@ class UniqueSet V8_FINAL : public ZoneObject {
 
   // Add a new element to this unique set. Mutates this set. O(|this|).
   void Add(Unique<T> uniq, Zone* zone) {
+    ASSERT(uniq.IsInitialized());
     // Keep the set sorted by the {raw_address} of the unique elements.
     for (int i = 0; i < size_; i++) {
       if (array_[i] == uniq) return;

src/x64/lithium-codegen-x64.cc

@@ -4876,7 +4876,7 @@ void LCodeGen::DoCheckInstanceType(LCheckInstanceType* instr) {
 
 void LCodeGen::DoCheckValue(LCheckValue* instr) {
   Register reg = ToRegister(instr->value());
-  Handle<HeapObject> object = instr->hydrogen()->object();
+  Handle<HeapObject> object = instr->hydrogen()->object().handle();
   __ CmpHeapObject(reg, object);
   DeoptimizeIf(not_equal, instr->environment());
 }
@@ -4917,22 +4917,21 @@ void LCodeGen::DoCheckMaps(LCheckMaps* instr) {
   ASSERT(input->IsRegister());
   Register reg = ToRegister(input);
 
-  SmallMapList* map_set = instr->hydrogen()->map_set();
-
   DeferredCheckMaps* deferred = NULL;
   if (instr->hydrogen()->has_migration_target()) {
     deferred = new(zone()) DeferredCheckMaps(this, instr, reg);
     __ bind(deferred->check_maps());
   }
 
+  UniqueSet<Map> map_set = instr->hydrogen()->map_set();
   Label success;
-  for (int i = 0; i < map_set->length() - 1; i++) {
-    Handle<Map> map = map_set->at(i);
+  for (int i = 0; i < map_set.size() - 1; i++) {
+    Handle<Map> map = map_set.at(i).handle();
     __ CompareMap(reg, map, &success);
     __ j(equal, &success);
   }
 
-  Handle<Map> map = map_set->last();
+  Handle<Map> map = map_set.at(map_set.size() - 1).handle();
   __ CompareMap(reg, map, &success);
   if (instr->hydrogen()->has_migration_target()) {
     __ j(not_equal, deferred->entry());