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

vm/object.cc

 // Copyright (c) 2011, the Dart project authors.  Please see the AUTHORS file
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
 
 #include "vm/object.h"
 
 #include "vm/assembler.h"
 #include "vm/assert.h"
 #include "vm/bigint_operations.h"
 #include "vm/bootstrap.h"
@@ skipping 4993 matching lines @@
       if ((*reinterpret_cast<RawObject**>(this_addr + offset)) !=
           (*reinterpret_cast<RawObject**>(other_addr + offset))) {
         return false;
       }
     }
   }
   return true;
 }
 
 
+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:

     const Class& cls = Class::Handle(this->clazz());
     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.
     Instance& norm_value = Instance::Handle();
     intptr_t index = 0;
     while (index < constants_len) {
       norm_value ^= constants.At(index);
       if (norm_value.IsNull()) {
         break;
       }
       if (this->Equals(norm_value)) {
         return norm_value.raw();
       }
       index++;
     }
     // 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();
 }
 
 
 RawType* Instance::GetType() const {
   if (IsNull()) {
     return Type::NullType();
   }
@@ skipping 317 matching lines @@
   {
     RawObject* raw = Object::Allocate(cls, Mint::InstanceSize(), space);
     NoGCScope no_gc;
     result ^= raw;
   }
   result.set_value(val);
   return result.raw();
 }
 
 
+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.
     return true;
   }
 
   if (!other.IsMint() || other.IsNull()) {
     return false;
   }
 
@@ skipping 50 matching lines @@
   OS::SNPrint(chars, len, kFormat, value());
   return chars;
 }
 
 
 void Double::set_value(double value) const {
   raw_ptr()->value_ = value;
 }
 
 
+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 =
       Class::Handle(isolate->object_store()->double_class());
   Double& result = Double::Handle();
   {
     RawObject* raw = Object::Allocate(cls, Double::InstanceSize(), space);
     NoGCScope no_gc;
     result ^= raw;
   }
   result.set_value(d);
   return result.raw();
 }
 
 
 static bool IsWhiteSpace(char ch) {
   return ch == '\0' || ch == '\n' || ch == '\r' || ch == ' ' || ch == '\t';
 }
 
 
+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_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) {
-  // 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);
-  // We do not treat overflow or underflow as an error and therefore do not
-  // check errno for ERANGE.
-  if (!IsWhiteSpace(*endptr)) {
+  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";
   }
   if (isinf(value())) {
     return value() < 0 ? "-Infinity" : "Infinity";
   }
   const char* kFormat = "%f";
   // Calculate the size of the string.
   intptr_t len = OS::SNPrint(NULL, 0, kFormat, value()) + 1;
@@ skipping 1805 matching lines @@
   const String& str = String::Handle(pattern());
   const char* format = "JSRegExp: pattern=%s flags=%s";
   intptr_t len = OS::SNPrint(NULL, 0, format, str.ToCString(), Flags());
   char* chars = reinterpret_cast<char*>(
       Isolate::Current()->current_zone()->Allocate(len + 1));
   OS::SNPrint(chars, (len + 1), format, str.ToCString(), Flags());
   return chars;
 }
 
 }  // namespace dart