Reviewers: jochen, ulan,

Message:
This shows some improvement (up to ~5%) for some of the Octane benchmarks (up to
~5% for Raytrace and Richards). The impact depends on the CPU benchmarked.

Ideally the same result should be achieved by modifying HConstant::EmitAtUses(),
but an attempt to do so triggered crashes.
For example trying:

--- a/src/hydrogen-instructions.cc
+++ b/src/hydrogen-instructions.cc
@@ -2870,6 +2870,7 @@ bool HConstant::EmitAtUses() {
   if (IsCell()) return false;
   if (representation().IsDouble()) return false;
   if (representation().IsExternal()) return false;
+  if (representation().IsTagged()) return false;
   return true;
 }

yields

  # Fatal error in ../src/arm64/lithium-codegen-arm64.cc, line 1856
  # CHECK(!info()->IsStub()) failed

Any pointer or ideas on what may be going wrong?

Description:
ARM64: Avoid regeneration of constants.

At the lithium level, HConstants are cached and their virtual register reused to
avoid regenerating them.

Please review this at https://codereview.chromium.org/308313002/

SVN Base: https://v8.googlecode.com/svn/branches/bleeding_edge

Affected files (+67, -1 lines):
  M src/arm64/lithium-arm64.h
  M src/arm64/lithium-arm64.cc
  M src/hydrogen-osr.h


Index: src/arm64/lithium-arm64.cc
diff --git a/src/arm64/lithium-arm64.cc b/src/arm64/lithium-arm64.cc
index 02127066536aa8dead082292d77f04a50b4f21de..6f60e8ae0d7b7f436a7800155d59326fd9b81b32 100644
--- a/src/arm64/lithium-arm64.cc
+++ b/src/arm64/lithium-arm64.cc
@@ -375,7 +375,12 @@ LUnallocated* LChunkBuilder::ToUnallocated(DoubleRegister reg) {
 LOperand* LChunkBuilder::Use(HValue* value, LUnallocated* operand) {
   if (value->EmitAtUses()) {
     HInstruction* instr = HInstruction::cast(value);
-    VisitInstruction(instr);
+    if (value->IsConstant() && IsCachedConstant(HConstant::cast(value),
+                                                current_block_)) {
+      // We will reuse the cached value.
+    } else {
+      VisitInstruction(instr);
+    }
   }
   operand->set_virtual_register(value->id());
   return operand;
@@ -586,7 +591,13 @@ void LChunkBuilder::DoBasicBlock(HBasicBlock* block) {
   ASSERT(is_building());
   current_block_ = block;

+  if (graph()->has_osr() &&
+      (block == graph()->osr()->osr_entry())) {
+    constant_cache_.Clear();
+  }
+
   if (block->IsStartBlock()) {
+    constant_cache_.Clear();
     block->UpdateEnvironment(graph_->start_environment());
     argument_count_ = 0;
   } else if (block->predecessors()->length() == 1) {
@@ -1328,6 +1339,7 @@ LInstruction* LChunkBuilder::DoCompareMap(HCompareMap* instr) {


 LInstruction* LChunkBuilder::DoConstant(HConstant* instr) {
+  CacheConstant(instr, current_block_);
   Representation r = instr->representation();
   if (r.IsSmi()) {
     return DefineAsRegister(new(zone()) LConstantS);
Index: src/arm64/lithium-arm64.h
diff --git a/src/arm64/lithium-arm64.h b/src/arm64/lithium-arm64.h
index be053ddbbda026ab18a97d07433d04b43f8fbcc3..fc6e667c6061295788eea765721d57f7616cff96 100644
--- a/src/arm64/lithium-arm64.h
+++ b/src/arm64/lithium-arm64.h
@@ -3012,6 +3012,50 @@ class LWrapReceiver V8_FINAL : public LTemplateInstruction<1, 2, 0> {
 };


+class ConstantCache {
+ public:
+  ConstantCache() {}
+
+  class ConstantCacheLevel {
+   public:
+    void Cache(HConstant* constant) { cached_constants_.insert(constant); }
+    bool IsCached(HConstant* constant) {
+      return cached_constants_.find(constant) != cached_constants_.end();
+    }
+
+   private:
+    std::set<HConstant*> cached_constants_;
+  };
+
+  void Clear() { cache_levels_.clear(); }
+
+  void Cache(HConstant* constant, HBasicBlock* block) {
+    cache_levels_[block].Cache(constant);
+  }
+
+  bool IsCached(HConstant* constant, HBasicBlock* block) {
+    HBasicBlock* base_block = block;
+    std::map<HBasicBlock*, ConstantCacheLevel>::iterator it;
+    while (block != NULL) {
+      it = cache_levels_.find(block);
+      if (it != cache_levels_.end() && it->second.IsCached(constant)) {
+        if (base_block != block) {
+ // Avoid traversing the dominator blocks multiple times for the same
+          // constant.
+          cache_levels_[block].Cache(constant);
+        }
+        return true;
+      }
+      block = block->dominator();
+    }
+    return false;
+  }
+
+ private:
+  std::map<HBasicBlock*, ConstantCacheLevel> cache_levels_;
+};
+
+
 class LChunkBuilder;
 class LPlatformChunk V8_FINAL : public LChunk {
  public:
@@ -3055,6 +3099,13 @@ class LChunkBuilder V8_FINAL : public LChunkBuilderBase {

   static bool HasMagicNumberForDivision(int32_t divisor);

+  bool IsCachedConstant(HConstant* constant, HBasicBlock* block) {
+    return constant_cache_.IsCached(constant, block);
+  }
+  void CacheConstant(HConstant* constant, HBasicBlock* block) {
+    constant_cache_.Cache(constant, block);
+  }
+
  private:
   enum Status {
     UNUSED,
@@ -3205,6 +3256,7 @@ class LChunkBuilder V8_FINAL : public LChunkBuilderBase {
   HInstruction* current_instruction_;
   HBasicBlock* current_block_;
   LAllocator* allocator_;
+  ConstantCache constant_cache_;

   DISALLOW_COPY_AND_ASSIGN(LChunkBuilder);
 };
Index: src/hydrogen-osr.h
diff --git a/src/hydrogen-osr.h b/src/hydrogen-osr.h
index 00bb2d4e1966ef2a0389490e752a51e86e42cd01..a1009ba2c06f2168f7a8b9b36beb1b99000b0024 100644
--- a/src/hydrogen-osr.h
+++ b/src/hydrogen-osr.h
@@ -41,6 +41,8 @@ class HOsrBuilder : public ZoneObject {

   bool HasOsrEntryAt(IterationStatement* statement);

+  HBasicBlock* osr_entry() { return osr_entry_; }
+
  private:
   int unoptimized_frame_slots_;
   HOptimizedGraphBuilder* builder_;


--
--
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
--- You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to v8-dev+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Reply via email to