[llvm] r317040 - Re-commit: [globalisel][tablegen] Keep track of the insertion point while adding BuildMIAction's. NFC

Daniel Sanders via llvm-commits llvm-commits at lists.llvm.org
Tue Oct 31 14:34:53 PDT 2017


Author: dsanders
Date: Tue Oct 31 14:34:53 2017
New Revision: 317040

URL: http://llvm.org/viewvc/llvm-project?rev=317040&view=rev
Log:
Re-commit: [globalisel][tablegen] Keep track of the insertion point while adding BuildMIAction's. NFC

Multi-instruction emission needs to ensure the the instructions are generated
a depth-first fashion. For example:
 (ADDWrr (SUBWrr a, b), c)
needs to emit the SUBWrr before the ADDWrr. However, our walk over
TreePatternNode's is highly context sensitive which makes it difficult to append
BuildMIActions in the order we want. To fix this, we now keep track of the
insertion point as we add actions. This will allow multi-insn emission to insert
BuildMI's in the correct place.

The previous commit failed on the Ubuntu bots using GCC 4.8. These bots didn't
like a call to emplace(). I've replaced it with insert() to see if it's a quirk
of the C++11 support.


Modified:
    llvm/trunk/utils/TableGen/GlobalISelEmitter.cpp

Modified: llvm/trunk/utils/TableGen/GlobalISelEmitter.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/utils/TableGen/GlobalISelEmitter.cpp?rev=317040&r1=317039&r2=317040&view=diff
==============================================================================
--- llvm/trunk/utils/TableGen/GlobalISelEmitter.cpp (original)
+++ llvm/trunk/utils/TableGen/GlobalISelEmitter.cpp Tue Oct 31 14:34:53 2017
@@ -517,6 +517,11 @@ class MatchAction;
 
 /// Generates code to check that a match rule matches.
 class RuleMatcher {
+public:
+  using ActionVec = std::vector<std::unique_ptr<MatchAction>>;
+  using const_action_iterator = ActionVec::const_iterator;
+
+protected:
   /// A list of matchers that all need to succeed for the current rule to match.
   /// FIXME: This currently supports a single match position but could be
   /// extended to support multiple positions to support div/rem fusion or
@@ -525,7 +530,7 @@ class RuleMatcher {
 
   /// A list of actions that need to be taken when all predicates in this rule
   /// have succeeded.
-  std::vector<std::unique_ptr<MatchAction>> Actions;
+  ActionVec Actions;
 
   using DefinedInsnVariablesMap =
       std::map<const InstructionMatcher *, unsigned>;
@@ -571,6 +576,9 @@ public:
   const std::vector<Record *> &getRequiredFeatures() const;
 
   template <class Kind, class... Args> Kind &addAction(Args &&... args);
+  template <class Kind, class... Args>
+  const_action_iterator insertAction(const_action_iterator InsertPt,
+                                     Args &&... args);
 
   /// Define an instruction without emitting any code to do so.
   /// This is used for the root of the match.
@@ -606,6 +614,12 @@ public:
     (void)R;
   }
 
+  const_action_iterator actions_begin() const { return Actions.begin(); }
+  const_action_iterator actions_end() const { return Actions.end(); }
+  iterator_range<const_action_iterator> actions() const {
+    return make_range(actions_begin(), actions_end());
+  }
+
   void defineOperand(StringRef SymbolicName, OperandMatcher &OM);
 
   void defineComplexSubOperand(StringRef SymbolicName, Record *ComplexPattern,
@@ -642,6 +656,8 @@ public:
   InstructionMatcher &insnmatcher_front() const { return *Matchers.front(); }
 };
 
+using const_action_iterator = RuleMatcher::const_action_iterator;
+
 template <class PredicateTy> class PredicateListMatcher {
 private:
   typedef std::vector<std::unique_ptr<PredicateTy>> PredicateVec;
@@ -1942,11 +1958,30 @@ const std::vector<Record *> &RuleMatcher
   return RequiredFeatures;
 }
 
+// Emplaces an action of the specified Kind at the end of the action list.
+//
+// Returns a reference to the newly created action.
+//
+// Like std::vector::emplace_back(), may invalidate all iterators if the new
+// size exceeds the capacity. Otherwise, only invalidates the past-the-end
+// iterator.
 template <class Kind, class... Args>
 Kind &RuleMatcher::addAction(Args &&... args) {
   Actions.emplace_back(llvm::make_unique<Kind>(std::forward<Args>(args)...));
   return *static_cast<Kind *>(Actions.back().get());
 }
+// Emplaces an action of the specified Kind before the given insertion point.
+//
+// Returns an iterator pointing at the newly created instruction.
+//
+// Like std::vector::insert(), may invalidate all iterators if the new size
+// exceeds the capacity. Otherwise, only invalidates the iterators from the
+// insertion point onwards.
+template <class Kind, class... Args>
+const_action_iterator RuleMatcher::insertAction(const_action_iterator InsertPt,
+                                                Args &&... args) {
+  return Actions.insert(InsertPt, llvm::make_unique<Kind>(std::forward<Args>(args)...));
+}
 
 unsigned
 RuleMatcher::implicitlyDefineInsnVar(const InstructionMatcher &Matcher) {
@@ -2225,15 +2260,18 @@ private:
   Expected<BuildMIAction &>
   createAndImportInstructionRenderer(RuleMatcher &M,
                                      const TreePatternNode *Dst);
-  Expected<BuildMIAction &>
-  createInstructionRenderer(RuleMatcher &M, const TreePatternNode *Dst);
+  Expected<const_action_iterator>
+  createInstructionRenderer(const_action_iterator InsertPt, RuleMatcher &M,
+                            const TreePatternNode *Dst);
   void importExplicitDefRenderers(BuildMIAction &DstMIBuilder);
-  Expected<BuildMIAction &>
-  importExplicitUseRenderers(RuleMatcher &M, BuildMIAction &DstMIBuilder,
+  Expected<const_action_iterator>
+  importExplicitUseRenderers(const_action_iterator InsertPt, RuleMatcher &M,
+                             BuildMIAction &DstMIBuilder,
                              const llvm::TreePatternNode *Dst);
-  Error importExplicitUseRenderer(RuleMatcher &Rule,
-                                  BuildMIAction &DstMIBuilder,
-                                  TreePatternNode *DstChild) const;
+  Expected<const_action_iterator>
+  importExplicitUseRenderer(const_action_iterator InsertPt, RuleMatcher &Rule,
+                            BuildMIAction &DstMIBuilder,
+                            TreePatternNode *DstChild) const;
   Error importDefaultOperandRenderers(BuildMIAction &DstMIBuilder,
                                       DagInit *DefaultOps) const;
   Error
@@ -2563,9 +2601,9 @@ Error GlobalISelEmitter::importChildMatc
   return failedImport("Src pattern child is an unsupported kind");
 }
 
-Error GlobalISelEmitter::importExplicitUseRenderer(
-    RuleMatcher &Rule, BuildMIAction &DstMIBuilder,
-    TreePatternNode *DstChild) const {
+Expected<const_action_iterator> GlobalISelEmitter::importExplicitUseRenderer(
+    const_action_iterator InsertPt, RuleMatcher &Rule,
+    BuildMIAction &DstMIBuilder, TreePatternNode *DstChild) const {
   if (DstChild->getTransformFn() != nullptr) {
     return failedImport("Dst pattern child has transform fn " +
                         DstChild->getTransformFn()->getName());
@@ -2576,7 +2614,7 @@ Error GlobalISelEmitter::importExplicitU
     DstMIBuilder.addRenderer<RenderComplexPatternOperand>(
         0, *std::get<0>(*SubOperand), DstChild->getName(),
         std::get<1>(*SubOperand), std::get<2>(*SubOperand));
-    return Error::success();
+    return InsertPt;
   }
 
   if (!DstChild->isLeaf()) {
@@ -2586,7 +2624,7 @@ Error GlobalISelEmitter::importExplicitU
       auto &ChildSDNI = CGP.getSDNodeInfo(DstChild->getOperator());
       if (ChildSDNI.getSDClassName() == "BasicBlockSDNode") {
         DstMIBuilder.addRenderer<CopyRenderer>(0, DstChild->getName());
-        return Error::success();
+        return InsertPt;
       }
     }
 
@@ -2597,11 +2635,11 @@ Error GlobalISelEmitter::importExplicitU
     if (DstChild->getOperator()->getName() == "imm") {
       DstMIBuilder.addRenderer<CopyConstantAsImmRenderer>(0,
                                                           DstChild->getName());
-      return Error::success();
+      return InsertPt;
     } else if (DstChild->getOperator()->getName() == "fpimm") {
       DstMIBuilder.addRenderer<CopyFConstantAsFPImmRenderer>(
           0, DstChild->getName());
-      return Error::success();
+      return InsertPt;
     }
 
     return failedImport("Dst pattern child isn't a leaf node or an MBB" + llvm::to_string(*DstChild));
@@ -2623,7 +2661,7 @@ Error GlobalISelEmitter::importExplicitU
 
     if (ChildRec->isSubClassOf("Register")) {
       DstMIBuilder.addRenderer<AddRegisterRenderer>(0, ChildRec);
-      return Error::success();
+      return InsertPt;
     }
 
     if (ChildRec->isSubClassOf("RegisterClass") ||
@@ -2633,11 +2671,11 @@ Error GlobalISelEmitter::importExplicitU
           !ChildRec->isValueUnset("GIZeroRegister")) {
         DstMIBuilder.addRenderer<CopyOrAddZeroRegRenderer>(
             0, DstChild->getName(), ChildRec->getValueAsDef("GIZeroRegister"));
-        return Error::success();
+        return InsertPt;
       }
 
       DstMIBuilder.addRenderer<CopyRenderer>(0, DstChild->getName());
-      return Error::success();
+      return InsertPt;
     }
 
     if (ChildRec->isSubClassOf("ComplexPattern")) {
@@ -2650,7 +2688,7 @@ Error GlobalISelEmitter::importExplicitU
       DstMIBuilder.addRenderer<RenderComplexPatternOperand>(
           0, *ComplexPattern->second, DstChild->getName(),
           OM.getAllocatedTemporariesBaseID());
-      return Error::success();
+      return InsertPt;
     }
 
     if (ChildRec->isSubClassOf("SDNodeXForm"))
@@ -2666,22 +2704,26 @@ Error GlobalISelEmitter::importExplicitU
 
 Expected<BuildMIAction &> GlobalISelEmitter::createAndImportInstructionRenderer(
     RuleMatcher &M, const TreePatternNode *Dst) {
-  auto DstMIBuilderOrError = createInstructionRenderer(M, Dst);
-  if (auto Error = DstMIBuilderOrError.takeError())
+  auto InsertPtOrError = createInstructionRenderer(M.actions_end(), M, Dst);
+  if (auto Error = InsertPtOrError.takeError())
     return std::move(Error);
 
-  BuildMIAction &DstMIBuilder = DstMIBuilderOrError.get();
+  const_action_iterator InsertPt = InsertPtOrError.get();
+  BuildMIAction &DstMIBuilder = *static_cast<BuildMIAction *>(InsertPt->get());
 
   importExplicitDefRenderers(DstMIBuilder);
 
-  if (auto Error = importExplicitUseRenderers(M, DstMIBuilder, Dst).takeError())
+  if (auto Error = importExplicitUseRenderers(InsertPt, M, DstMIBuilder, Dst)
+                       .takeError())
     return std::move(Error);
 
   return DstMIBuilder;
 }
 
-Expected<BuildMIAction &> GlobalISelEmitter::createInstructionRenderer(
-    RuleMatcher &M, const TreePatternNode *Dst) {
+Expected<const_action_iterator>
+GlobalISelEmitter::createInstructionRenderer(const_action_iterator InsertPt,
+                                             RuleMatcher &M,
+                                             const TreePatternNode *Dst) {
   Record *DstOp = Dst->getOperator();
   if (!DstOp->isSubClassOf("Instruction")) {
     if (DstOp->isSubClassOf("ValueType"))
@@ -2698,9 +2740,9 @@ Expected<BuildMIAction &> GlobalISelEmit
   else if (DstI->TheDef->getName() == "EXTRACT_SUBREG")
     DstI = &Target.getInstruction(RK.getDef("COPY"));
 
-  auto &DstMIBuilder = M.addAction<BuildMIAction>(0, DstI);
+  InsertPt = M.insertAction<BuildMIAction>(InsertPt, 0, DstI);
 
-  return DstMIBuilder;
+  return InsertPt;
 }
 
 void GlobalISelEmitter::importExplicitDefRenderers(
@@ -2712,8 +2754,8 @@ void GlobalISelEmitter::importExplicitDe
   }
 }
 
-Expected<BuildMIAction &> GlobalISelEmitter::importExplicitUseRenderers(
-    RuleMatcher &M, BuildMIAction &DstMIBuilder,
+Expected<const_action_iterator> GlobalISelEmitter::importExplicitUseRenderers(
+    const_action_iterator InsertPt, RuleMatcher &M, BuildMIAction &DstMIBuilder,
     const llvm::TreePatternNode *Dst) {
   const CodeGenInstruction *DstI = DstMIBuilder.getCGI();
   CodeGenInstruction *OrigDstI = &Target.getInstruction(Dst->getOperator());
@@ -2739,7 +2781,7 @@ Expected<BuildMIAction &> GlobalISelEmit
 
       DstMIBuilder.addRenderer<CopySubRegRenderer>(
           0, Dst->getChild(0)->getName(), SubIdx);
-      return DstMIBuilder;
+      return InsertPt;
     }
 
     return failedImport("EXTRACT_SUBREG child #1 is not a subreg index");
@@ -2772,9 +2814,11 @@ Expected<BuildMIAction &> GlobalISelEmit
       continue;
     }
 
-    if (auto Error =
-            importExplicitUseRenderer(M, DstMIBuilder, Dst->getChild(Child)))
+    auto InsertPtOrError = importExplicitUseRenderer(InsertPt, M, DstMIBuilder,
+                                                     Dst->getChild(Child));
+    if (auto Error = InsertPtOrError.takeError())
       return std::move(Error);
+    InsertPt = InsertPtOrError.get();
     ++Child;
   }
 
@@ -2785,7 +2829,7 @@ Expected<BuildMIAction &> GlobalISelEmit
                         " explicit ones and " + llvm::to_string(NumDefaultOps) +
                         " default ones");
 
-  return DstMIBuilder;
+  return InsertPt;
 }
 
 Error GlobalISelEmitter::importDefaultOperandRenderers(




More information about the llvm-commits mailing list