[PATCH] D113475: [llvm-tblgen][RISCV] Make llvm-tblgen RISCVCompressInstEmitter to be common infra across different targets

[Fraser Cormack via Phabricator via llvm-commits]

frasercrmck added inline comments.


================
Comment at: llvm/include/llvm/Target/Target.td:663
 
+class CompressPat<dag input, dag output, list<Predicate> predicates = []> {
+  dag Input  = input;
----------------
We should probably comment this class and its fields. At least direct users to the `CompressInstEmitter` backend.


================
Comment at: llvm/utils/TableGen/CompressInstEmitter.cpp:27
+//   dag Output    = output;
+//   list<Predicate> Predicates = [];
+// }
----------------
`isCompressOnly` missing from this description.


================
Comment at: llvm/utils/TableGen/CompressInstEmitter.cpp:96
+    std::vector<Record *>
+        PatReqFeatures; // Required target features to enable pattern.
+    IndexedMap<OpData>
----------------
I know you've inherited this from the RISCV version, but I'm not a fan of the same-line commenting: it messes up the formatting. Could they be on the lines above?


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

https://reviews.llvm.org/D113475