Defer creating Handles for HConstants to the code generation phase.

src/hydrogen-instructions.h

 // Copyright 2012 the V8 project authors. All rights reserved.
 // Redistribution and use in source and binary forms, with or without
 // modification, are permitted provided that the following conditions are
 // met:
 //
 //     * Redistributions of source code must retain the above copyright
 //       notice, this list of conditions and the following disclaimer.
 //     * Redistributions in binary form must reproduce the above
 //       copyright notice, this list of conditions and the following
 //       disclaimer in the documentation and/or other materials provided
@@ skipping 2435 matching lines @@
     return Representation::None();
   }
 
   DECLARE_CONCRETE_INSTRUCTION(ArgumentsObject)
 };
 
 
 class HConstant: public HTemplateInstruction<0> {
  public:
   HConstant(Handle<Object> handle, Representation r);
+  HConstant(int32_t value, Representation r);
+  HConstant(double value, Representation r);
 
-  Handle<Object> handle() const { return handle_; }
+  Handle<Object> handle() {
+    if (handle_.is_null()) {
+      handle_ = FACTORY->NewNumber(double_value_, TENURED);
+    }
+    ASSERT(has_int32_value_ || !handle_->IsSmi());
+    return handle_;
+  }
 
   bool InOldSpace() const { return !HEAP->InNewSpace(*handle_); }
 
   bool ImmortalImmovable() const {
+    if (has_int32_value_) {
+      return false;
+    }
+    if (has_double_value_) {
+      if (BitCast<int64_t>(double_value_) == BitCast<int64_t>(-0.0) ||
+          isnan(double_value_)) {
+        return true;
+      }
+      return false;
+    }
+
+    ASSERT(!handle_.is_null());
     Heap* heap = HEAP;
+    // We should have handled minus_zero_value and nan_value in the
+    // has_double_value_ clause above.
+    ASSERT(*handle_ != heap->minus_zero_value());
+    ASSERT(*handle_ != heap->nan_value());
     if (*handle_ == heap->undefined_value()) return true;
     if (*handle_ == heap->null_value()) return true;
     if (*handle_ == heap->true_value()) return true;
     if (*handle_ == heap->false_value()) return true;
     if (*handle_ == heap->the_hole_value()) return true;
-    if (*handle_ == heap->minus_zero_value()) return true;
-    if (*handle_ == heap->nan_value()) return true;
     if (*handle_ == heap->empty_string()) return true;
     return false;
   }
 
   virtual Representation RequiredInputRepresentation(int index) {
     return Representation::None();
   }
 
+  bool IsTagged() {

[Michael Starzinger, 2012/07/11 09:16:10]

This method doesn't seem to be used at all. Let's drop it.

[sanjoy, 2012/07/11 10:54:08]

Done.

+    ASSERT(has_int32_value_ || has_double_value_ || !handle_.is_null());
+    return !(has_int32_value_ || has_double_value_);
+  }
+
   virtual bool IsConvertibleToInteger() const {
-    if (handle_->IsSmi()) return true;
-    if (handle_->IsHeapNumber() &&
-        (HeapNumber::cast(*handle_)->value() ==
-         static_cast<double>(NumberToInt32(*handle_)))) return true;
-    return false;
+    return has_int32_value_;
   }
 
   virtual bool EmitAtUses() { return !representation().IsDouble(); }
   virtual HValue* Canonicalize();
   virtual void PrintDataTo(StringStream* stream);
   virtual HType CalculateInferredType();
-  bool IsInteger() const { return handle_->IsSmi(); }
+  bool IsInteger() { return handle()->IsSmi(); }
   HConstant* CopyToRepresentation(Representation r, Zone* zone) const;
   HConstant* CopyToTruncatedInt32(Zone* zone) const;
   bool HasInteger32Value() const { return has_int32_value_; }
   int32_t Integer32Value() const {
     ASSERT(HasInteger32Value());
     return int32_value_;
   }
   bool HasDoubleValue() const { return has_double_value_; }
   double DoubleValue() const {
     ASSERT(HasDoubleValue());
     return double_value_;
   }
-  bool HasNumberValue() const { return has_int32_value_ || has_double_value_; }
+  bool HasNumberValue() const { return has_double_value_; }
   int32_t NumberValueAsInteger32() const {
     ASSERT(HasNumberValue());
-    if (has_int32_value_) return int32_value_;
-    return DoubleToInt32(double_value_);
+    return int32_value_;

[Michael Starzinger, 2012/07/11 09:16:10]

I think "int32_value_" is only valid if "HasInteger32Value()" returns true. So
either the assertion or the method implementation is wrong.

I guess this depends on the fact that the constructors set int32_value_
correctly even though has_int32_value_ is set to false. If that's the case, then
a comment right above this return statement that explains this is necessary.

[sanjoy, 2012/07/11 10:54:08]

Done.

   }
-  bool HasStringValue() const { return handle_->IsString(); }
 
-  bool ToBoolean() const;
+  bool ToBoolean();
 
   virtual intptr_t Hashcode() {
     ASSERT(!HEAP->allow_allocation(false));
-    intptr_t hash = reinterpret_cast<intptr_t>(*handle());
-    // Prevent smis from having fewer hash values when truncated to
-    // the least significant bits.
-    const int kShiftSize = kSmiShiftSize + kSmiTagSize;
-    STATIC_ASSERT(kShiftSize != 0);
-    return hash ^ (hash >> kShiftSize);
+    intptr_t hash;
+
+    if (has_int32_value_) {
+      hash = static_cast<intptr_t>(int32_value_);
+      // Prevent smis from having fewer hash values when truncated to
+      // the least significant bits.
+      const int kShiftSize = kSmiShiftSize + kSmiTagSize;

[Michael Starzinger, 2012/07/11 09:16:10]

Since we are using the untagged int32 value here, both the shifting and the
comment are no longer necessary.

[sanjoy, 2012/07/11 10:54:08]

Done.

+      STATIC_ASSERT(kShiftSize != 0);
+      hash = hash ^ (hash >> kShiftSize);
+    } else if (has_double_value_) {
+      hash = static_cast<intptr_t>(BitCast<int64_t>(double_value_));
+    } else {
+      ASSERT(!handle_.is_null());
+      hash = reinterpret_cast<intptr_t>(*handle_);
+    }
+
+    return hash;
   }
 
 #ifdef DEBUG
   virtual void Verify() { }
 #endif
 
   DECLARE_CONCRETE_INSTRUCTION(Constant)
 
  protected:
   virtual Range* InferRange(Zone* zone);
 
   virtual bool DataEquals(HValue* other) {
     HConstant* other_constant = HConstant::cast(other);
-    return handle().is_identical_to(other_constant->handle());
+    if (has_int32_value_) {
+      return other_constant->has_int32_value_ &&
+          int32_value_ == other_constant->int32_value_;
+    } else if (has_double_value_) {
+      return other_constant->has_double_value_ &&
+          BitCast<int64_t>(double_value_) ==
+          BitCast<int64_t>(other_constant->double_value_);
+    } else {
+      ASSERT(!handle_.is_null());
+      return !other_constant->handle_.is_null() &&
+          *handle_ == *other_constant->handle_;
+    }
   }
 
  private:
+  // If this is a numerical constant, handle_ either points to to the
+  // HeapObject the constant originated from or is null.  If the
+  // constant is non-numeric, handle_ always points to a valid
+  // constant HeapObject.
   Handle<Object> handle_;
 
-  // The following two values represent the int32 and the double value of the
-  // given constant if there is a lossless conversion between the constant
-  // and the specific representation.
+  // We store the HConstant in the most specific form safely possible.
+  // The two flags, has_int32_value_ and has_double_value_ tell us if
+  // int32_value_ and double_value_ hold valid, safe representations
+  // of the constant.  has_int32_value_ implies has_double_value_ but
+  // not the converse.
   bool has_int32_value_ : 1;
   bool has_double_value_ : 1;
   int32_t int32_value_;
   double double_value_;
 };
 
 
 class HBinaryOperation: public HTemplateInstruction<3> {
  public:
   HBinaryOperation(HValue* context, HValue* left, HValue* right) {
@@ skipping 2550 matching lines @@
   DECLARE_CONCRETE_INSTRUCTION(LoadFieldByIndex);
 };
 
 
 #undef DECLARE_INSTRUCTION
 #undef DECLARE_CONCRETE_INSTRUCTION
 
 } }  // namespace v8::internal
 
 #endif  // V8_HYDROGEN_INSTRUCTIONS_H_