[PATCH] D154945: [TableGen] Enable multiple instructions in patterns

[Craig Topper via Phabricator via llvm-commits]

craig.topper added inline comments.


================
Comment at: llvm/utils/TableGen/CodeGenDAGPatterns.cpp:3766
 }
+/// Get all the instructions in all trees in a pattern.
+static void getInstructionsInPattern(const TreePattern &Pat,
----------------
Add blank line between functions


================
Comment at: llvm/utils/TableGen/CodeGenDAGPatterns.cpp:4309
+      for (const TypeSetByHwMode &TypeSet : OutTree->getExtTypes()) {
+        FlatOutTypes.push_back(TypeSet);
       }
----------------
You can avoid the loop with something like
```
const std::vector<TypeSetByHwMode> Types = OutTree->getExtTypes();
FlatOutTypes.insert(FlatOutTypes.end(), Types.begin(), Types.end());
```


================
Comment at: llvm/utils/TableGen/CodeGenDAGPatterns.h:926
+  /// Construct from multiple patterns
+  TreePattern(Record *TheRec, std::vector<TreePatternNodePtr> &Pat,
+              bool isInput, CodeGenDAGPatterns &ise);
----------------
`const std::vector<TreePatternNodePtr> &`


================
Comment at: llvm/utils/TableGen/DAGISelEmitter.cpp:40
 
-/// getResultPatternCost - Compute the number of instructions for this pattern.
+/// Compute the number of instructions for this tree.
 /// This is a temporary hack.  We should really include the instruction
----------------
Can these changes be pre-committed?


================
Comment at: llvm/utils/TableGen/DAGISelEmitter.cpp:71
+/// Compute the code size of instructions for this tree.
+static unsigned getResultPatternSize(const TreePatternNode *P,
+                                     const CodeGenDAGPatterns &CGP) {
----------------
Can the changes to this function be pre-commited?


================
Comment at: llvm/utils/TableGen/DAGISelMatcherGen.cpp:796
+  // Is this the root of the first/last tree in the output TreePattern?
+  const bool isFirstRoot = N == Pattern.getDstPattern().getTrees().front();
+  const bool isLastRoot = N == Pattern.getDstPattern().getTrees().back();
----------------
Can you capitalize the variable namings per the coding standards?


================
Comment at: llvm/utils/TableGen/DAGISelMatcherGen.cpp:945
   // gets the excess operands from the input DAG.
+  const bool srcIsVariadic =
+      Pattern.getSrcPattern()->NodeHasProperty(SDNPVariadic, CGP);
----------------
Capitalize


================
Comment at: llvm/utils/TableGen/DAGISelMatcherGen.cpp:1107
+  for (const auto &Tree : Pattern.getDstPattern().getTrees()) {
+    for (unsigned i = 0; i < Tree->getNumTypes(); ++i) {
+      Results[ResBase + i] = Ops[ResBase + Tree->getResultIndex(i)];
----------------
Drop curly braces


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D154945/new/

https://reviews.llvm.org/D154945