Add Double::NewCanonical and Mint::NewCanonical so that it is possible to

vm/object.cc

Index: vm/object.cc
===================================================================
--- vm/object.cc	(revision 2470)
+++ vm/object.cc	(working copy)
@@ -5011,6 +5011,24 @@
 }
 
 
+static void InsertCanonicalConstant(const Class& cls,
+                                    const Array& canonical_list,
+                                    intptr_t index,
+                                    const Instance& constant) {

[srdjan, 2011/12/15 22:02:26]

Why not make it a Class function?

[siva, 2011/12/15 23:42:54]

Done.

+  // The constant needs to be added to the list. Grow the list if it is full.
+  const intptr_t list_len = canonical_list.Length();
+  if (index == list_len) {

[srdjan, 2011/12/15 22:02:26]

index >= list_len ?

[siva, 2011/12/15 23:42:54]

Done.

+    const intptr_t new_length = (list_len == 0) ? 4 : list_len * 2;

[srdjan, 2011/12/15 22:02:26]

That grows a little bit too quickly, IMHO.

[siva, 2011/12/15 23:42:54]

Changed it to (list_len + 4) for growth, so grow by 4 elements a time.

On 2011/12/15 22:02:26, srdjan wrote:

+    const Array& new_canonical_list =
+        Array::Handle(Array::Grow(canonical_list, new_length, Heap::kOld));
+    cls.set_constants(new_canonical_list);
+    new_canonical_list.SetAt(index, constant);
+  } else {
+    canonical_list.SetAt(index, constant);
+  }
+}
+
+
 RawInstance* Instance::Canonicalize() const {
   ASSERT(!IsNull());
   if (!IsCanonical()) {

[srdjan, 2011/12/15 22:02:26]

Can we move all this code into Class, thus no access here to Class::constants()?

[siva, 2011/12/15 23:42:54]

As discussed off-line, I have added a TODO to consider
refactoring this so thath we can effectively share for
all look ups.

On 2011/12/15 22:02:26, srdjan wrote:

@@ -5034,18 +5052,7 @@
     // The value needs to be added to the list. Grow the list if
     // it is full.
     // TODO(srdjan): Copy instance into old space if canonicalized?
-    if (index == constants_len) {
-      const intptr_t kInitialConstLength = 4;
-      const intptr_t old_length = constants.Length();
-      const intptr_t new_length =
-          (old_length == 0) ? kInitialConstLength : old_length * 2;
-      const Array& new_constants =
-          Array::Handle(Array::Grow(constants, new_length, Heap::kOld));
-      cls.set_constants(new_constants);
-      new_constants.SetAt(index, *this);
-    } else {
-      constants.SetAt(index, *this);
-    }
+    InsertCanonicalConstant(cls, constants, index, *this);
     SetCanonical();
   }
   return this->raw();
@@ -5383,6 +5390,36 @@
 }
 
 
+RawMint* Mint::NewCanonical(int64_t value) {
+  // Do not allocate a Mint if Smi would do.
+  ASSERT(!Smi::IsValid64(value));

[srdjan, 2011/12/15 22:02:26]

If we move this code into Class, we could share it with others?

[siva, 2011/12/15 23:42:54]

Ditto.

On 2011/12/15 22:02:26, srdjan wrote:

+  const Class& cls =
+      Class::Handle(Isolate::Current()->object_store()->mint_class());

[srdjan, 2011/12/15 22:02:26]

Isn't cls = Class::Handle(clazz()); ? 

[siva, 2011/12/15 23:42:54]

This is a static method, clazz() is not valid.

On 2011/12/15 22:02:26, srdjan wrote:

+  const Array& constants = Array::Handle(cls.constants());
+  const intptr_t constants_len = constants.Length();
+  // Linear search to see whether this value is already present in the
+  // list of canonicalized constants.
+  Mint& canonical_value = Mint::Handle();
+  intptr_t index = 0;
+  while (index < constants_len) {
+    canonical_value ^= constants.At(index);
+    if (canonical_value.IsNull()) {
+      break;
+    }
+    if (canonical_value.value() == value) {
+      return canonical_value.raw();
+    }
+    index++;
+  }
+  // The value needs to be added to the constants list. Grow the list if
+  // it is full.
+  canonical_value = Mint::New(value, Heap::kOld);
+  InsertCanonicalConstant(cls, constants, index, canonical_value);
+  canonical_value.SetCanonical();
+  return canonical_value.raw();
+}
+
+
 bool Mint::Equals(const Instance& other) const {
   if (this->raw() == other.raw()) {
     // Both handles point to the same raw instance.
@@ -5453,6 +5490,28 @@
 }
 
 
+bool Double::Equals(double value) const {
+  intptr_t value_offset = Double::value_offset();
+  void* this_addr = reinterpret_cast<void*>(
+      reinterpret_cast<uword>(this->raw_ptr()) + value_offset);
+  void* other_addr = reinterpret_cast<void*>(&value);
+  return memcmp(this_addr, other_addr, sizeof(value)) == 0;
+}

[srdjan, 2011/12/15 22:02:26]

Can you use (*reinterpret_cast<int64_t*>(&value) to do the bitwise compare?

[siva, 2011/12/15 23:42:54]

If I do that I get :
error: dereferencing type-punned pointer will break strict-aliasing rules

On 2011/12/15 22:02:26, srdjan wrote:

+
+
+bool Double::Equals(const Instance& other) const {
+  if (this->raw() == other.raw()) {
+    return true;  // "===".
+  }
+  if (other.IsNull() || !other.IsDouble()) {
+    return false;
+  }
+  Double& other_dbl = Double::Handle();
+  other_dbl ^= other.raw();
+  return Equals(other_dbl.value());
+}
+
+
 RawDouble* Double::New(double d, Heap::Space space) {
   Isolate* isolate = Isolate::Current();
   const Class& cls =
@@ -5473,20 +5532,67 @@
 }
 
 
-RawDouble* Double::New(const String& str, Heap::Space space) {
+static bool StringToDouble(const String& str, double* double_value) {
+  ASSERT(double_value != NULL);
   // TODO(regis): For now, we use strtod to convert a string to double.
   const char* nptr = str.ToCString();
   char* endptr = NULL;
-  double double_value = strtod(nptr, &endptr);
+  *double_value = strtod(nptr, &endptr);
   // We do not treat overflow or underflow as an error and therefore do not
   // check errno for ERANGE.
   if (!IsWhiteSpace(*endptr)) {
+    return false;
+  }
+  return true;
+}
+
+
+RawDouble* Double::New(const String& str, Heap::Space space) {
+  double double_value;
+  if (!StringToDouble(str, &double_value)) {
     return Double::Handle().raw();
   }
   return New(double_value, space);
 }
 
 
+RawDouble* Double::NewCanonical(double value) {
+  const Class& cls =
+      Class::Handle(Isolate::Current()->object_store()->double_class());
+  const Array& constants = Array::Handle(cls.constants());
+  const intptr_t constants_len = constants.Length();
+  // Linear search to see whether this value is already present in the
+  // list of canonicalized constants.
+  Double& canonical_value = Double::Handle();
+  intptr_t index = 0;
+  while (index < constants_len) {
+    canonical_value ^= constants.At(index);
+    if (canonical_value.IsNull()) {
+      break;
+    }
+    if (canonical_value.Equals(value)) {
+      return canonical_value.raw();
+    }
+    index++;
+  }
+  // The value needs to be added to the constants list. Grow the list if
+  // it is full.
+  canonical_value = Double::New(value, Heap::kOld);
+  InsertCanonicalConstant(cls, constants, index, canonical_value);
+  canonical_value.SetCanonical();
+  return canonical_value.raw();
+}
+
+
+RawDouble* Double::NewCanonical(const String& str) {
+  double double_value;
+  if (!StringToDouble(str, &double_value)) {
+    return Double::Handle().raw();
+  }
+  return NewCanonical(double_value);
+}
+
+
 const char* Double::ToCString() const {
   if (isnan(value())) {
     return "NaN";