From d5afdbe5a456c055df19421c8d1810d65fbfaeee Mon Sep 17 00:00:00 2001
From: James Molloy <jmolloy@google.com>
Date: Fri, 18 Oct 2019 14:48:35 +0000
Subject: [PATCH] [DFAPacketizer] Fix large compile-time regression for VLIW
 targets

D68992 / rL375086 refactored the packetizer and removed a bunch of logic. Unfortunately it creates an Automaton object whenever a DFAPacketizer is required. These objects have no longevity, and in particular on a debug build the population of the Automaton's transition map from the underlying table is very slow (because it is called ~10 times per MachineFunction, in the testcase I'm looking at).

This patch changes Automaton to wrap its underlying constant data in std::shared_ptr, which allows trivial copy construction. The DFAPacketizer creation function now creates a static archetypical Automaton and copies that whenever a new DFAPacketizer is required.

This takes a testcase down from ~20s to ~0.5s in debug mode.

llvm-svn: 375240
---
 llvm/include/llvm/Support/Automaton.h        | 23 ++++++++++++--------
 llvm/utils/TableGen/DFAPacketizerEmitter.cpp |  4 ++--
 2 files changed, 16 insertions(+), 11 deletions(-)

diff --git a/llvm/include/llvm/Support/Automaton.h b/llvm/include/llvm/Support/Automaton.h
index a5a4aef0130b..7c13a698e492 100644
--- a/llvm/include/llvm/Support/Automaton.h
+++ b/llvm/include/llvm/Support/Automaton.h
@@ -161,10 +161,13 @@ template <typename ActionT> class Automaton {
   /// FIXME: This uses a std::map because ActionT can be a pair type including
   /// an enum. In particular DenseMapInfo<ActionT> must be defined to use
   /// DenseMap here.
-  std::map<std::pair<uint64_t, ActionT>, std::pair<uint64_t, unsigned>> M;
+  /// This is a shared_ptr to allow very quick copy-construction of Automata; this
+  /// state is immutable after construction so this is safe.
+  using MapTy = std::map<std::pair<uint64_t, ActionT>, std::pair<uint64_t, unsigned>>;
+  std::shared_ptr<MapTy> M;
   /// An optional transcription object. This uses much more state than simply
   /// traversing the DFA for acceptance, so is heap allocated.
-  std::unique_ptr<internal::NfaTranscriber> Transcriber;
+  std::shared_ptr<internal::NfaTranscriber> Transcriber;
   /// The initial DFA state is 1.
   uint64_t State = 1;
   /// True if we should transcribe and false if not (even if Transcriber is defined).
@@ -187,13 +190,15 @@ public:
             ArrayRef<NfaStatePair> TranscriptionTable = {}) {
     if (!TranscriptionTable.empty())
       Transcriber =
-          std::make_unique<internal::NfaTranscriber>(TranscriptionTable);
+          std::make_shared<internal::NfaTranscriber>(TranscriptionTable);
     Transcribe = Transcriber != nullptr;
+    M = std::make_shared<MapTy>();
     for (const auto &I : Transitions)
       // Greedily read and cache the transition table.
-      M.emplace(std::make_pair(I.FromDfaState, I.Action),
-                std::make_pair(I.ToDfaState, I.InfoIdx));
+      M->emplace(std::make_pair(I.FromDfaState, I.Action),
+                 std::make_pair(I.ToDfaState, I.InfoIdx));
   }
+  Automaton(const Automaton &) = default;
 
   /// Reset the automaton to its initial state.
   void reset() {
@@ -218,8 +223,8 @@ public:
   /// If this function returns false, all methods are undefined until reset() is
   /// called.
   bool add(const ActionT &A) {
-    auto I = M.find({State, A});
-    if (I == M.end())
+    auto I = M->find({State, A});
+    if (I == M->end())
       return false;
     if (Transcriber && Transcribe)
       Transcriber->transition(I->second.second);
@@ -229,8 +234,8 @@ public:
 
   /// Return true if the automaton can be transitioned based on input symbol A.
   bool canAdd(const ActionT &A) {
-    auto I = M.find({State, A});
-    return I != M.end();
+    auto I = M->find({State, A});
+    return I != M->end();
   }
 
   /// Obtain a set of possible paths through the input nondeterministic
diff --git a/llvm/utils/TableGen/DFAPacketizerEmitter.cpp b/llvm/utils/TableGen/DFAPacketizerEmitter.cpp
index 0d3c6fdc933c..ccb4ef1b9678 100644
--- a/llvm/utils/TableGen/DFAPacketizerEmitter.cpp
+++ b/llvm/utils/TableGen/DFAPacketizerEmitter.cpp
@@ -538,10 +538,10 @@ void DFAPacketizerEmitter::emitForItineraries(
   OS << "DFAPacketizer *" << SubTargetClassName << "::"
      << "create" << DFAName
      << "DFAPacketizer(const InstrItineraryData *IID) const {\n"
-     << "  Automaton<uint64_t> A(ArrayRef<" << TargetAndDFAName
+     << "  static Automaton<uint64_t> A(ArrayRef<" << TargetAndDFAName
      << "Transition>(" << TargetAndDFAName << "Transitions), "
      << TargetAndDFAName << "TransitionInfo);\n"
-     << "  return new DFAPacketizer(IID, std::move(A));\n"
+     << "  return new DFAPacketizer(IID, A);\n"
      << "\n}\n\n";
 }