From: jochen@chromium.org Date: Tue, 12 Aug 2014 12:53:14 +0000 (+0000) Subject: Revert 23077 - "Use CommonNodeCache for heap constants in ChangeLowering." X-Git-Tag: upstream/4.7.83~7647 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=5f1f897894519f7cdde3185a90745d1b912f280e;p=platform%2Fupstream%2Fv8.git 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 --- diff --git a/src/compiler/change-lowering.cc b/src/compiler/change-lowering.cc index 8f3be18..3f8e45b 100644 --- a/src/compiler/change-lowering.cc +++ b/src/compiler/change-lowering.cc @@ -34,11 +34,8 @@ Node* ChangeLoweringBase::ExternalConstant(ExternalReference reference) { Node* ChangeLoweringBase::HeapConstant(PrintableUnique 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)); } diff --git a/src/compiler/common-node-cache.h b/src/compiler/common-node-cache.h index f9a832b..2b0ac0b 100644 --- a/src/compiler/common-node-cache.h +++ b/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(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(value)); } - Node** FindHeapConstant(PrintableUnique 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_ diff --git a/src/compiler/node-cache.cc b/src/compiler/node-cache.cc index 0d79c76..c3ee58c 100644 --- a/src/compiler/node-cache.cc +++ b/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 -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(key)); +} + + +template <> +inline int32_t NodeCacheHash(void* key) { + return ComputePointerHash(key); +} + + template bool NodeCache::Resize(Zone* zone) { if (size_ >= max_) return false; // Don't grow past the maximum size. @@ -64,7 +76,7 @@ bool NodeCache::Resize(Zone* zone) { template Node** NodeCache::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::Find(Zone* zone, Key key) { } -template class NodeCache; template class NodeCache; - -} // namespace compiler -} // namespace internal -} // namespace v8 +template class NodeCache; +template class NodeCache; +} +} +} // namespace v8::internal::compiler diff --git a/src/compiler/node-cache.h b/src/compiler/node-cache.h index 0a5f3ef..35352ea 100644 --- a/src/compiler/node-cache.h +++ b/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 @@ -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 Int32NodeCache; typedef NodeCache Int64NodeCache; -typedef NodeCache IntPtrNodeCache; - -} // namespace compiler -} // namespace internal -} // namespace v8 +typedef NodeCache Int32NodeCache; +typedef NodeCache PtrNodeCache; +} +} +} // namespace v8::internal::compiler #endif // V8_COMPILER_NODE_CACHE_H_ diff --git a/test/cctest/cctest.gyp b/test/cctest/cctest.gyp index 9d57cc0..42946f5 100644 --- a/test/cctest/cctest.gyp +++ b/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', diff --git a/test/cctest/compiler/test-node-cache.cc b/test/cctest/compiler/test-node-cache.cc new file mode 100644 index 0000000..23909a5 --- /dev/null +++ b/test/cctest/compiler/test-node-cache.cc @@ -0,0 +1,160 @@ +// 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(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(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(kSize); + CommonOperatorBuilder common(graph.zone()); + + for (int i = 0; i < kSize; i++) { + int64_t v = static_cast(i) * static_cast(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(i) * static_cast(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(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(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); +} diff --git a/test/compiler-unittests/common-node-cache-unittest.cc b/test/compiler-unittests/common-node-cache-unittest.cc deleted file mode 100644 index 5d5f3fe..0000000 --- a/test/compiler-unittests/common-node-cache-unittest.cc +++ /dev/null @@ -1,125 +0,0 @@ -// 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 n = PrintableUnique::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 t = PrintableUnique::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 diff --git a/test/compiler-unittests/compiler-unittests.gyp b/test/compiler-unittests/compiler-unittests.gyp index 0b62207..c1de0c4 100644 --- a/test/compiler-unittests/compiler-unittests.gyp +++ b/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', ], diff --git a/test/compiler-unittests/node-cache-unittest.cc b/test/compiler-unittests/node-cache-unittest.cc deleted file mode 100644 index 2f67b23..0000000 --- a/test/compiler-unittests/node-cache-unittest.cc +++ /dev/null @@ -1,129 +0,0 @@ -// 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 - -#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 -class NodeCacheTest : public CompilerTest { - public: - NodeCacheTest() : rng_(FLAG_random_seed) {} - virtual ~NodeCacheTest() {} - - protected: - NodeCache* 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 cache_; - base::RandomNumberGenerator rng_; -}; - - -template <> -int32_t NodeCacheTest::GenerateRandom() { - return rng()->NextInt(); -} - - -template <> -int64_t NodeCacheTest::GenerateRandom() { - int64_t v; - rng()->NextBytes(&v, sizeof(v)); - return v; -} - - -typedef ::testing::Types 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(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(kSize); - this->GenerateRandom(&values[0], &values[kSize]); - Node* nodes = this->zone()->template NewArray(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 diff --git a/testing/gtest-type-names.h b/testing/gtest-type-names.h index ab9fa61..ba900dd 100644 --- a/testing/gtest-type-names.h +++ b/testing/gtest-type-names.h @@ -11,10 +11,10 @@ namespace testing { namespace internal { -#define GET_TYPE_NAME(type) \ - template <> \ - inline std::string GetTypeName() { \ - return #type; \ +#define GET_TYPE_NAME(type) \ + template <> \ + std::string GetTypeName() { \ + return #type; \ } GET_TYPE_NAME(int8_t) GET_TYPE_NAME(uint8_t)