[llvm] r375240 - [DFAPacketizer] Fix large compile-time regression for VLIW targets

[James Molloy via llvm-commits]

Author: jamesm
Date: Fri Oct 18 07:48:35 2019
New Revision: 375240

URL: http://llvm.org/viewvc/llvm-project?rev=375240&view=rev
Log:
[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.

Modified:
    llvm/trunk/include/llvm/Support/Automaton.h
    llvm/trunk/utils/TableGen/DFAPacketizerEmitter.cpp

Modified: llvm/trunk/include/llvm/Support/Automaton.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Support/Automaton.h?rev=375240&r1=375239&r2=375240&view=diff
==============================================================================
--- llvm/trunk/include/llvm/Support/Automaton.h (original)
+++ llvm/trunk/include/llvm/Support/Automaton.h Fri Oct 18 07:48:35 2019
@@ -161,10 +161,13 @@ template <typename ActionT> class Automa
   /// 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

Modified: llvm/trunk/utils/TableGen/DFAPacketizerEmitter.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/utils/TableGen/DFAPacketizerEmitter.cpp?rev=375240&r1=375239&r2=375240&view=diff
==============================================================================
--- llvm/trunk/utils/TableGen/DFAPacketizerEmitter.cpp (original)
+++ llvm/trunk/utils/TableGen/DFAPacketizerEmitter.cpp Fri Oct 18 07:48:35 2019
@@ -538,10 +538,10 @@ void DFAPacketizerEmitter::emitForItiner
   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";
 }