Revert 23077 - "Use CommonNodeCache for heap constants in ChangeLowering."

Breaks compilation on Mac64

| TEST=compiler-unittests
| R=jarin@chromium.org
|
| Committed: https://code.google.com/p/v8/source/detail?r=23077

TBR=bmeurer@chromium.org,jarin@chromium.org
LOG=n
BUG=none

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

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

src/compiler/change-lowering.cc

@@ -34,11 +34,8 @@ Node* ChangeLoweringBase::ExternalConstant(ExternalReference reference) {
 
 
 Node* ChangeLoweringBase::HeapConstant(PrintableUnique<HeapObject> value) {
-  Node** loc = cache()->FindHeapConstant(value);
-  if (*loc == NULL) {
-    *loc = graph()->NewNode(common()->HeapConstant(value));
-  }
-  return *loc;
+  // TODO(bmeurer): Use common node cache.
+  return graph()->NewNode(common()->HeapConstant(value));
 }
 
 

src/compiler/common-node-cache.h

@@ -7,7 +7,6 @@
 
 #include "src/assembler.h"
 #include "src/compiler/node-cache.h"
-#include "src/unique.h"
 
 namespace v8 {
 namespace internal {
@@ -28,8 +27,7 @@ class CommonNodeCache V8_FINAL : public ZoneObject {
   }
 
   Node** FindExternalConstant(ExternalReference reference) {
-    return external_constants_.Find(
-        zone_, reinterpret_cast<intptr_t>(reference.address()));
+    return external_constants_.Find(zone_, reference.address());
   }
 
   Node** FindNumberConstant(double value) {
@@ -37,23 +35,17 @@ class CommonNodeCache V8_FINAL : public ZoneObject {
     return number_constants_.Find(zone_, BitCast<int64_t>(value));
   }
 
-  Node** FindHeapConstant(PrintableUnique<HeapObject> object) {
-    return heap_constants_.Find(zone_, object.Hashcode());
-  }
+  Zone* zone() const { return zone_; }
 
  private:
   Int32NodeCache int32_constants_;
   Int64NodeCache float64_constants_;
-  IntPtrNodeCache external_constants_;
+  PtrNodeCache external_constants_;
   Int64NodeCache number_constants_;
-  IntPtrNodeCache heap_constants_;
   Zone* zone_;
-
-  DISALLOW_COPY_AND_ASSIGN(CommonNodeCache);
 };
-
-}  // namespace compiler
-}  // namespace internal
-}  // namespace v8
+}
+}
+}  // namespace v8::internal::compiler
 
 #endif  // V8_COMPILER_COMMON_NODE_CACHE_H_

src/compiler/node-cache.cc

@@ -4,8 +4,6 @@
 
 #include "src/compiler/node-cache.h"
 
-#include "src/zone.h"
-
 namespace v8 {
 namespace internal {
 namespace compiler {
@@ -14,21 +12,35 @@ namespace compiler {
 #define LINEAR_PROBE 5
 
 template <typename Key>
-inline int NodeCacheHash(Key key);
-
+int32_t NodeCacheHash(Key key) {
+  UNIMPLEMENTED();
+  return 0;
+}
 
 template <>
-inline int NodeCacheHash(int32_t key) {
+inline int32_t NodeCacheHash(int32_t key) {
   return ComputeIntegerHash(key, 0);
 }
 
 
 template <>
-inline int NodeCacheHash(int64_t key) {
+inline int32_t NodeCacheHash(int64_t key) {
   return ComputeLongHash(key);
 }
 
 
+template <>
+inline int32_t NodeCacheHash(double key) {
+  return ComputeLongHash(BitCast<int64_t>(key));
+}
+
+
+template <>
+inline int32_t NodeCacheHash(void* key) {
+  return ComputePointerHash(key);
+}
+
+
 template <typename Key>
 bool NodeCache<Key>::Resize(Zone* zone) {
   if (size_ >= max_) return false;  // Don't grow past the maximum size.
@@ -64,7 +76,7 @@ bool NodeCache<Key>::Resize(Zone* zone) {
 
 template <typename Key>
 Node** NodeCache<Key>::Find(Zone* zone, Key key) {
-  int hash = NodeCacheHash(key);
+  int32_t hash = NodeCacheHash(key);
   if (entries_ == NULL) {
     // Allocate the initial entries and insert the first entry.
     int num_entries = INITIAL_SIZE + LINEAR_PROBE;
@@ -100,9 +112,9 @@ Node** NodeCache<Key>::Find(Zone* zone, Key key) {
 }
 
 
-template class NodeCache<int32_t>;
 template class NodeCache<int64_t>;
-
-}  // namespace compiler
-}  // namespace internal
-}  // namespace v8
+template class NodeCache<int32_t>;
+template class NodeCache<void*>;
+}
+}
+}  // namespace v8::internal::compiler

src/compiler/node-cache.h

@@ -5,19 +5,14 @@
 #ifndef V8_COMPILER_NODE_CACHE_H_
 #define V8_COMPILER_NODE_CACHE_H_
 
-#include "src/base/macros.h"
+#include "src/v8.h"
+
+#include "src/compiler/node.h"
 
 namespace v8 {
 namespace internal {
-
-// Forward declarations.
-class Zone;
-
 namespace compiler {
 
-// Forward declarations.
-class Node;
-
 // A cache for nodes based on a key. Useful for implementing canonicalization of
 // nodes such as constants, parameters, etc.
 template <typename Key>
@@ -41,21 +36,18 @@ class NodeCache {
   };
 
   Entry* entries_;  // lazily-allocated hash entries.
-  int size_;
-  int max_;
+  int32_t size_;
+  int32_t max_;
 
   bool Resize(Zone* zone);
-
-  DISALLOW_COPY_AND_ASSIGN(NodeCache);
 };
 
 // Various default cache types.
-typedef NodeCache<int32_t> Int32NodeCache;
 typedef NodeCache<int64_t> Int64NodeCache;
-typedef NodeCache<intptr_t> IntPtrNodeCache;
-
-}  // namespace compiler
-}  // namespace internal
-}  // namespace v8
+typedef NodeCache<int32_t> Int32NodeCache;
+typedef NodeCache<void*> PtrNodeCache;
+}
+}
+}  // namespace v8::internal::compiler
 
 #endif  // V8_COMPILER_NODE_CACHE_H_

test/cctest/cctest.gyp

@@ -65,6 +65,7 @@
         'compiler/test-linkage.cc',
         'compiler/test-machine-operator-reducer.cc',
         'compiler/test-node-algorithm.cc',
+        'compiler/test-node-cache.cc',
         'compiler/test-node.cc',
         'compiler/test-operator.cc',
         'compiler/test-phi-reducer.cc',

test/cctest/compiler/test-node-cache.cc

+// Copyright 2014 the V8 project authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "src/v8.h"
+
+#include "graph-tester.h"
+#include "src/compiler/common-operator.h"
+#include "src/compiler/node-cache.h"
+
+using namespace v8::internal;
+using namespace v8::internal::compiler;
+
+TEST(Int32Constant_back_to_back) {
+  GraphTester graph;
+  Int32NodeCache cache;
+
+  for (int i = -2000000000; i < 2000000000; i += 3315177) {
+    Node** pos = cache.Find(graph.zone(), i);
+    CHECK_NE(NULL, pos);
+    for (int j = 0; j < 3; j++) {
+      Node** npos = cache.Find(graph.zone(), i);
+      CHECK_EQ(pos, npos);
+    }
+  }
+}
+
+
+TEST(Int32Constant_five) {
+  GraphTester graph;
+  Int32NodeCache cache;
+  CommonOperatorBuilder common(graph.zone());
+
+  int32_t constants[] = {static_cast<int32_t>(0x80000000), -77, 0, 1, -1};
+
+  Node* nodes[ARRAY_SIZE(constants)];
+
+  for (size_t i = 0; i < ARRAY_SIZE(constants); i++) {
+    int32_t k = constants[i];
+    Node* node = graph.NewNode(common.Int32Constant(k));
+    *cache.Find(graph.zone(), k) = nodes[i] = node;
+  }
+
+  for (size_t i = 0; i < ARRAY_SIZE(constants); i++) {
+    int32_t k = constants[i];
+    CHECK_EQ(nodes[i], *cache.Find(graph.zone(), k));
+  }
+}
+
+
+TEST(Int32Constant_hits) {
+  GraphTester graph;
+  Int32NodeCache cache;
+  const int32_t kSize = 1500;
+  Node** nodes = graph.zone()->NewArray<Node*>(kSize);
+  CommonOperatorBuilder common(graph.zone());
+
+  for (int i = 0; i < kSize; i++) {
+    int32_t v = i * -55;
+    nodes[i] = graph.NewNode(common.Int32Constant(v));
+    *cache.Find(graph.zone(), v) = nodes[i];
+  }
+
+  int hits = 0;
+  for (int i = 0; i < kSize; i++) {
+    int32_t v = i * -55;
+    Node** pos = cache.Find(graph.zone(), v);
+    if (*pos != NULL) {
+      CHECK_EQ(nodes[i], *pos);
+      hits++;
+    }
+  }
+  CHECK_LT(4, hits);
+}
+
+
+TEST(Int64Constant_back_to_back) {
+  GraphTester graph;
+  Int64NodeCache cache;
+
+  for (int64_t i = -2000000000; i < 2000000000; i += 3315177) {
+    Node** pos = cache.Find(graph.zone(), i);
+    CHECK_NE(NULL, pos);
+    for (int j = 0; j < 3; j++) {
+      Node** npos = cache.Find(graph.zone(), i);
+      CHECK_EQ(pos, npos);
+    }
+  }
+}
+
+
+TEST(Int64Constant_hits) {
+  GraphTester graph;
+  Int64NodeCache cache;
+  const int32_t kSize = 1500;
+  Node** nodes = graph.zone()->NewArray<Node*>(kSize);
+  CommonOperatorBuilder common(graph.zone());
+
+  for (int i = 0; i < kSize; i++) {
+    int64_t v = static_cast<int64_t>(i) * static_cast<int64_t>(5003001);
+    nodes[i] = graph.NewNode(common.Int32Constant(i));
+    *cache.Find(graph.zone(), v) = nodes[i];
+  }
+
+  int hits = 0;
+  for (int i = 0; i < kSize; i++) {
+    int64_t v = static_cast<int64_t>(i) * static_cast<int64_t>(5003001);
+    Node** pos = cache.Find(graph.zone(), v);
+    if (*pos != NULL) {
+      CHECK_EQ(nodes[i], *pos);
+      hits++;
+    }
+  }
+  CHECK_LT(4, hits);
+}
+
+
+TEST(PtrConstant_back_to_back) {
+  GraphTester graph;
+  PtrNodeCache cache;
+  int32_t buffer[50];
+
+  for (int32_t* p = buffer;
+       (p - buffer) < static_cast<ptrdiff_t>(ARRAY_SIZE(buffer)); p++) {
+    Node** pos = cache.Find(graph.zone(), p);
+    CHECK_NE(NULL, pos);
+    for (int j = 0; j < 3; j++) {
+      Node** npos = cache.Find(graph.zone(), p);
+      CHECK_EQ(pos, npos);
+    }
+  }
+}
+
+
+TEST(PtrConstant_hits) {
+  GraphTester graph;
+  PtrNodeCache cache;
+  const int32_t kSize = 50;
+  int32_t buffer[kSize];
+  Node* nodes[kSize];
+  CommonOperatorBuilder common(graph.zone());
+
+  for (size_t i = 0; i < ARRAY_SIZE(buffer); i++) {
+    int k = static_cast<int>(i);
+    int32_t* p = &buffer[i];
+    nodes[i] = graph.NewNode(common.Int32Constant(k));
+    *cache.Find(graph.zone(), p) = nodes[i];
+  }
+
+  int hits = 0;
+  for (size_t i = 0; i < ARRAY_SIZE(buffer); i++) {
+    int32_t* p = &buffer[i];
+    Node** pos = cache.Find(graph.zone(), p);
+    if (*pos != NULL) {
+      CHECK_EQ(nodes[i], *pos);
+      hits++;
+    }
+  }
+  CHECK_LT(4, hits);
+}

test/compiler-unittests/common-node-cache-unittest.cc

-// Copyright 2014 the V8 project authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-#include "src/compiler/common-node-cache.h"
-#include "src/compiler/common-operator.h"
-#include "src/compiler/graph.h"
-#include "test/compiler-unittests/compiler-unittests.h"
-#include "testing/gmock/include/gmock/gmock.h"
-
-using testing::AllOf;
-using testing::IsNull;
-using testing::NotNull;
-using testing::Pointee;
-
-namespace v8 {
-namespace internal {
-namespace compiler {
-
-class CommonNodeCacheTest : public CompilerTest {
- public:
-  CommonNodeCacheTest() : cache_(zone()), common_(zone()), graph_(zone()) {}
-  virtual ~CommonNodeCacheTest() {}
-
- protected:
-  Factory* factory() const { return isolate()->factory(); }
-  CommonNodeCache* cache() { return &cache_; }
-  CommonOperatorBuilder* common() { return &common_; }
-  Graph* graph() { return &graph_; }
-
- private:
-  CommonNodeCache cache_;
-  CommonOperatorBuilder common_;
-  Graph graph_;
-};
-
-
-TEST_F(CommonNodeCacheTest, FindInt32Constant) {
-  Node** l42 = cache()->FindInt32Constant(42);
-  ASSERT_THAT(l42, AllOf(NotNull(), Pointee(IsNull())));
-  Node* n42 = *l42 = graph()->NewNode(common()->Int32Constant(42));
-
-  Node** l0 = cache()->FindInt32Constant(0);
-  ASSERT_THAT(l0, AllOf(NotNull(), Pointee(IsNull())));
-  Node* n0 = *l0 = graph()->NewNode(common()->Int32Constant(0));
-
-  EXPECT_THAT(cache()->FindInt32Constant(42), AllOf(l42, Pointee(n42)));
-  EXPECT_THAT(cache()->FindInt32Constant(0), AllOf(l0, Pointee(n0)));
-  EXPECT_THAT(cache()->FindInt32Constant(42), AllOf(l42, Pointee(n42)));
-  EXPECT_THAT(cache()->FindInt32Constant(0), AllOf(l0, Pointee(n0)));
-}
-
-
-TEST_F(CommonNodeCacheTest, FindFloat64Constant) {
-  Node** l42 = cache()->FindFloat64Constant(42.0);
-  ASSERT_THAT(l42, AllOf(NotNull(), Pointee(IsNull())));
-  Node* n42 = *l42 = graph()->NewNode(common()->Float64Constant(42.0));
-
-  Node** l0 = cache()->FindFloat64Constant(0.0);
-  ASSERT_THAT(l0, AllOf(NotNull(), Pointee(IsNull())));
-  Node* n0 = *l0 = graph()->NewNode(common()->Float64Constant(0.0));
-
-  EXPECT_THAT(cache()->FindFloat64Constant(42.0), AllOf(l42, Pointee(n42)));
-  EXPECT_THAT(cache()->FindFloat64Constant(0.0), AllOf(l0, Pointee(n0)));
-  EXPECT_THAT(cache()->FindFloat64Constant(42.0), AllOf(l42, Pointee(n42)));
-  EXPECT_THAT(cache()->FindFloat64Constant(0.0), AllOf(l0, Pointee(n0)));
-}
-
-
-TEST_F(CommonNodeCacheTest, FindExternalConstant) {
-  ExternalReference i = ExternalReference::isolate_address(isolate());
-  Node** li = cache()->FindExternalConstant(i);
-  ASSERT_THAT(li, AllOf(NotNull(), Pointee(IsNull())));
-  Node* ni = *li = graph()->NewNode(common()->ExternalConstant(i));
-
-  ExternalReference m = ExternalReference::address_of_min_int();
-  Node** lm = cache()->FindExternalConstant(m);
-  ASSERT_THAT(lm, AllOf(NotNull(), Pointee(IsNull())));
-  Node* nm = *lm = graph()->NewNode(common()->ExternalConstant(m));
-
-  EXPECT_THAT(cache()->FindExternalConstant(i), AllOf(li, Pointee(ni)));
-  EXPECT_THAT(cache()->FindExternalConstant(m), AllOf(lm, Pointee(nm)));
-  EXPECT_THAT(cache()->FindExternalConstant(i), AllOf(li, Pointee(ni)));
-  EXPECT_THAT(cache()->FindExternalConstant(m), AllOf(lm, Pointee(nm)));
-}
-
-
-TEST_F(CommonNodeCacheTest, FindNumberConstant) {
-  Node** l42 = cache()->FindNumberConstant(42.0);
-  ASSERT_THAT(l42, AllOf(NotNull(), Pointee(IsNull())));
-  Node* n42 = *l42 = graph()->NewNode(common()->NumberConstant(42.0));
-
-  Node** l0 = cache()->FindNumberConstant(0.0);
-  ASSERT_THAT(l0, AllOf(NotNull(), Pointee(IsNull())));
-  Node* n0 = *l0 = graph()->NewNode(common()->NumberConstant(0.0));
-
-  EXPECT_THAT(cache()->FindNumberConstant(42.0), AllOf(l42, Pointee(n42)));
-  EXPECT_THAT(cache()->FindNumberConstant(0.0), AllOf(l0, Pointee(n0)));
-  EXPECT_THAT(cache()->FindNumberConstant(42.0), AllOf(l42, Pointee(n42)));
-  EXPECT_THAT(cache()->FindNumberConstant(0.0), AllOf(l0, Pointee(n0)));
-}
-
-
-TEST_F(CommonNodeCacheTest, FindHeapConstant) {
-  PrintableUnique<HeapObject> n = PrintableUnique<HeapObject>::CreateImmovable(
-      zone(), factory()->null_value());
-  Node** ln = cache()->FindHeapConstant(n);
-  ASSERT_THAT(ln, AllOf(NotNull(), Pointee(IsNull())));
-  Node* nn = *ln = graph()->NewNode(common()->HeapConstant(n));
-
-  PrintableUnique<HeapObject> t = PrintableUnique<HeapObject>::CreateImmovable(
-      zone(), factory()->true_value());
-  Node** lt = cache()->FindHeapConstant(t);
-  ASSERT_THAT(lt, AllOf(NotNull(), Pointee(IsNull())));
-  Node* nt = *lt = graph()->NewNode(common()->HeapConstant(t));
-
-  EXPECT_THAT(cache()->FindHeapConstant(n), AllOf(ln, Pointee(nn)));
-  EXPECT_THAT(cache()->FindHeapConstant(t), AllOf(lt, Pointee(nt)));
-  EXPECT_THAT(cache()->FindHeapConstant(n), AllOf(ln, Pointee(nn)));
-  EXPECT_THAT(cache()->FindHeapConstant(t), AllOf(lt, Pointee(nt)));
-}
-
-}  // namespace compiler
-}  // namespace internal
-}  // namespace v8

test/compiler-unittests/compiler-unittests.gyp

@@ -21,10 +21,8 @@
       ],
       'sources': [  ### gcmole(all) ###
         'change-lowering-unittest.cc',
-        'common-node-cache-unittest.cc',
         'compiler-unittests.cc',
         'instruction-selector-unittest.cc',
-        'node-cache-unittest.cc',
         'node-matchers.cc',
         'node-matchers.h',
       ],

test/compiler-unittests/node-cache-unittest.cc

-// Copyright 2014 the V8 project authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-#include <algorithm>
-
-#include "src/base/utils/random-number-generator.h"
-#include "src/compiler/node.h"
-#include "src/compiler/node-cache.h"
-#include "src/flags.h"
-#include "test/compiler-unittests/compiler-unittests.h"
-#include "testing/gmock/include/gmock/gmock.h"
-#include "testing/gtest-type-names.h"
-
-using testing::AllOf;
-using testing::IsNull;
-using testing::NotNull;
-using testing::Pointee;
-
-namespace v8 {
-namespace internal {
-namespace compiler {
-
-template <typename T>
-class NodeCacheTest : public CompilerTest {
- public:
-  NodeCacheTest() : rng_(FLAG_random_seed) {}
-  virtual ~NodeCacheTest() {}
-
- protected:
-  NodeCache<T>* cache() { return &cache_; }
-  base::RandomNumberGenerator* rng() { return &rng_; }
-
-  void GenerateRandom(T* first, T* last) {
-    for (T* i = first; i != last; ++i) {
-      do {
-        *i = GenerateRandom();
-      } while (std::find(first, i, *i) != i);
-    }
-  }
-
- private:
-  T GenerateRandom();
-
-  NodeCache<T> cache_;
-  base::RandomNumberGenerator rng_;
-};
-
-
-template <>
-int32_t NodeCacheTest<int32_t>::GenerateRandom() {
-  return rng()->NextInt();
-}
-
-
-template <>
-int64_t NodeCacheTest<int64_t>::GenerateRandom() {
-  int64_t v;
-  rng()->NextBytes(&v, sizeof(v));
-  return v;
-}
-
-
-typedef ::testing::Types<int32_t, int64_t> NodeCacheTypes;
-TYPED_TEST_CASE(NodeCacheTest, NodeCacheTypes);
-
-
-TYPED_TEST(NodeCacheTest, BackToBack) {
-  static const size_t kSize = 100;
-  TypeParam values[kSize];
-  this->GenerateRandom(&values[0], &values[kSize]);
-  for (const TypeParam* i = &values[0]; i != &values[kSize]; ++i) {
-    TypeParam value = *i;
-    SCOPED_TRACE(::testing::Message() << "value " << value);
-    Node** location = this->cache()->Find(this->zone(), value);
-    ASSERT_THAT(location, AllOf(NotNull(), Pointee(IsNull())));
-    for (int attempt = 1; attempt < 4; ++attempt) {
-      SCOPED_TRACE(::testing::Message() << "attempt " << attempt);
-      EXPECT_EQ(location, this->cache()->Find(this->zone(), value));
-    }
-  }
-}
-
-
-TYPED_TEST(NodeCacheTest, MinimumSize) {
-  static const size_t kSize = 5;
-  TypeParam values[kSize];
-  this->GenerateRandom(&values[0], &values[kSize]);
-  Node** locations[kSize];
-  Node* nodes = this->zone()->template NewArray<Node>(kSize);
-  for (size_t i = 0; i < kSize; ++i) {
-    locations[i] = this->cache()->Find(this->zone(), values[i]);
-    ASSERT_THAT(locations[i], NotNull());
-    EXPECT_EQ(&locations[i],
-              std::find(&locations[0], &locations[i], locations[i]));
-    *locations[i] = &nodes[i];
-  }
-  for (size_t i = 0; i < kSize; ++i) {
-    EXPECT_EQ(locations[i], this->cache()->Find(this->zone(), values[i]));
-  }
-}
-
-
-TYPED_TEST(NodeCacheTest, MinimumHits) {
-  static const size_t kSize = 250;
-  static const size_t kMinHits = 10;
-  TypeParam* values = this->zone()->template NewArray<TypeParam>(kSize);
-  this->GenerateRandom(&values[0], &values[kSize]);
-  Node* nodes = this->zone()->template NewArray<Node>(kSize);
-  for (size_t i = 0; i < kSize; ++i) {
-    Node** location = this->cache()->Find(this->zone(), values[i]);
-    ASSERT_THAT(location, AllOf(NotNull(), Pointee(IsNull())));
-    *location = &nodes[i];
-  }
-  size_t hits = 0;
-  for (size_t i = 0; i < kSize; ++i) {
-    Node** location = this->cache()->Find(this->zone(), values[i]);
-    ASSERT_THAT(location, NotNull());
-    if (*location != NULL) {
-      EXPECT_EQ(&nodes[i], *location);
-      ++hits;
-    }
-  }
-  EXPECT_GE(hits, kMinHits);
-}
-
-}  // namespace compiler
-}  // namespace internal
-}  // namespace v8

testing/gtest-type-names.h

@@ -11,10 +11,10 @@
 namespace testing {
 namespace internal {
 
-#define GET_TYPE_NAME(type)                \
-  template <>                              \
-  inline std::string GetTypeName<type>() { \
-    return #type;                          \
+#define GET_TYPE_NAME(type)         \
+  template <>                       \
+  std::string GetTypeName<type>() { \
+    return #type;                   \
   }
 GET_TYPE_NAME(int8_t)
 GET_TYPE_NAME(uint8_t)