Reviewers: ulan, jochen,

Message:
This patch diminishes the regression observed at
https://codereview.chromium.org/209333004 .

With this patch applied, the timings for
make -j32 arm64.release.check TESTFLAGS=--time
TESTJOBS=mozilla/js1_5/Regress/regress-280769-2
become:

Without the regexp veneer patch:
   1 (00:06.818) mozilla/js1_5/Regress/regress-280769-2
   2 (00:01.910) mozilla/js1_5/Regress/regress-280769-2
   3 (00:01.711) mozilla/js1_5/Regress/regress-280769-2

With the regexp veneer patch:
   1 (00:11.922) mozilla/js1_5/Regress/regress-280769-2
   2 (00:04.014) mozilla/js1_5/Regress/regress-280769-2
   3 (00:04.014) mozilla/js1_5/Regress/regress-280769-2

Description:
ARM64: Avoid iterating through all unresolved branch infos when many are
pending.

Instead, inspect the label chain and delete pending information for every branch
in the chain.

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

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

Affected files (+75, -9 lines):
  M src/arm64/assembler-arm64.h
  M src/arm64/assembler-arm64.cc
  src/arm64/instructions-arm64.h


Index: src/arm64/assembler-arm64.cc
diff --git a/src/arm64/assembler-arm64.cc b/src/arm64/assembler-arm64.cc
index 5695623afd54e033ca6ec9ebfb349cef159b14c4..43bea9f160f8e0e32ed151fda5517e42be164dd5 100644
--- a/src/arm64/assembler-arm64.cc
+++ b/src/arm64/assembler-arm64.cc
@@ -545,6 +545,53 @@ int Assembler::LinkAndGetByteOffsetTo(Label* label) {
 }


+void Assembler::DeleteUnresolvedBranchInfoForLabelIterate(Label* label) {
+  std::multimap<int, FarBranchInfo>::iterator it_tmp, it;
+  it = unresolved_branches_.begin();
+  while (it != unresolved_branches_.end()) {
+    it_tmp = it++;
+    if (it_tmp->second.label_ == label) {
+      CHECK(it_tmp->first >= pc_offset());
+      unresolved_branches_.erase(it_tmp);
+    }
+  }
+}
+
+
+void Assembler::DeleteUnresolvedBranchInfoForLabelTraverse(Label* label) {
+  ASSERT(label->is_linked());
+  CheckLabelLinkChain(label);
+
+  int link_offset = label->pos();
+  int link_pcoffset;
+  bool end_of_chain = false;
+
+  while (!end_of_chain) {
+    Instruction * link = InstructionAt(link_offset);
+    link_pcoffset = link->ImmPCOffset();
+
+    // ADR instructions are not handled by veneers.
+    if (link->IsImmBranch()) {
+      int max_reachable_pc = InstructionOffset(link) +
+          Instruction::ImmBranchRange(link->BranchType());
+ typedef std::multimap<int, FarBranchInfo>::iterator unresolved_info_it;
+      std::pair<unresolved_info_it, unresolved_info_it> range;
+      range = unresolved_branches_.equal_range(max_reachable_pc);
+      unresolved_info_it it;
+      for (it = range.first; it != range.second; ++it) {
+        if (it->second.pc_offset_ == link_offset) {
+          unresolved_branches_.erase(it);
+          break;
+        }
+      }
+    }
+
+    end_of_chain = (link_pcoffset == 0);
+    link_offset = link_offset + link_pcoffset;
+  }
+}
+
+
 void Assembler::DeleteUnresolvedBranchInfoForLabel(Label* label) {
   if (unresolved_branches_.empty()) {
     ASSERT(next_veneer_pool_check_ == kMaxInt);
@@ -552,15 +599,17 @@ void Assembler::DeleteUnresolvedBranchInfoForLabel(Label* label) {
   }

   if (label->is_linked()) {
- // Branches to this label will be resolved when the label is bound below.
-    std::multimap<int, FarBranchInfo>::iterator it_tmp, it;
-    it = unresolved_branches_.begin();
-    while (it != unresolved_branches_.end()) {
-      it_tmp = it++;
-      if (it_tmp->second.label_ == label) {
-        CHECK(it_tmp->first >= pc_offset());
-        unresolved_branches_.erase(it_tmp);
-      }
+ // Branches to this label will be resolved when the label is bound, normally
+    // just after all the associated info has been deleted.
+
+    // If there are too many unresolved branches pending, avoid iterating
+    // through all of them to find the information to delete.
+    static const int kMaxNInfoIterate = 128;
+
+    if (unresolved_branches_.size() < kMaxNInfoIterate) {
+      DeleteUnresolvedBranchInfoForLabelIterate(label);
+    } else {
+      DeleteUnresolvedBranchInfoForLabelTraverse(label);
     }
   }
   if (unresolved_branches_.empty()) {
Index: src/arm64/assembler-arm64.h
diff --git a/src/arm64/assembler-arm64.h b/src/arm64/assembler-arm64.h
index 9a8cd467a0f7b8425c6dabe754f9f7f89cc54686..b0dd3b84addd1a32cd059d09a78646e292f3d08b 100644
--- a/src/arm64/assembler-arm64.h
+++ b/src/arm64/assembler-arm64.h
@@ -1694,6 +1694,10 @@ class Assembler : public AssemblerBase {
     return reinterpret_cast<Instruction*>(buffer_ + offset);
   }

+  ptrdiff_t InstructionOffset(Instruction* instr) const {
+    return reinterpret_cast<byte*>(instr) - buffer_;
+  }
+
   // Register encoding.
   static Instr Rd(CPURegister rd) {
     ASSERT(rd.code() != kSPRegInternalCode);
@@ -2182,6 +2186,15 @@ class Assembler : public AssemblerBase {
// not later attempt (likely unsuccessfully) to patch it to branch directly to
   // the label.
   void DeleteUnresolvedBranchInfoForLabel(Label* label);
+ // This function deletes the information related to the label by iterating + // through all unresolved branch infos and deleting the appropriating one. Its + // complexity is proportional to the number of unresolved branches pending.
+  void DeleteUnresolvedBranchInfoForLabelIterate(Label* label);
+ // This function deletes the information related to the label by traversing + // the label chain, and for each PC-relative instruction in the chain checking + // if pending unresolved information exists. Its complexity is proportional to
+  // the length of the label chain.
+  void DeleteUnresolvedBranchInfoForLabelTraverse(Label* label);

  private:
   PositionsRecorder positions_recorder_;
Index: src/arm64/instructions-arm64.h
diff --git a/src/arm64/instructions-arm64.h b/src/arm64/instructions-arm64.h
index de76b69b6b87120154041d5a7e250569900c4421..cfa63a345c81df784b592ae702fad1f8bf1a7c53 100644
--- a/src/arm64/instructions-arm64.h
+++ b/src/arm64/instructions-arm64.h
@@ -192,6 +192,10 @@ class Instruction {
     return Mask(TestBranchFMask) == TestBranchFixed;
   }

+  bool IsImmBranch() const {
+    return BranchType() != UnknownBranchType;
+  }
+
   bool IsLdrLiteral() const {
     return Mask(LoadLiteralFMask) == LoadLiteralFixed;
   }


--
--
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