[PATCH] D25614: [tablegen] Extract portions of AsmMatcherEmitter for re-use by another generator. NFC.

[Quentin Colombet via llvm-commits]

qcolombet accepted this revision.
qcolombet added a reviewer: qcolombet.
qcolombet added a comment.
This revision is now accepted and ready to land.

Hi Daniel,

LGTM. Couple of nitpicks below.

Cheers,
-Quentin



================
Comment at: utils/TableGen/AsmMatcherEmitter.cpp:1416
   // Build information about all of the AssemblerPredicates.
-  std::vector<Record*> AllPredicates =
-    Records.getAllDerivedDefinitions("Predicate");
-  for (Record *Pred : AllPredicates) {
-    // Ignore predicates that are not intended for the assembler.
-    if (!Pred->getValueAsBit("AssemblerMatcherPredicate"))
-      continue;
-
-    if (Pred->getName().empty())
-      PrintFatalError(Pred->getLoc(), "Predicate has no name!");
-
-    SubtargetFeatures.insert(std::make_pair(
-        Pred, SubtargetFeatureInfo(Pred, SubtargetFeatures.size())));
-    DEBUG(SubtargetFeatures.find(Pred)->second.dump());
-    assert(SubtargetFeatures.size() <= 64 && "Too many subtarget features!");
-  }
+  const auto &SubtargetFeaturePairs = SubtargetFeatureInfo::getAll(Records);
+  SubtargetFeatures.insert(SubtargetFeaturePairs.begin(),
----------------
I would rather stick to the explicit type here.


================
Comment at: utils/TableGen/SubtargetFeatureInfo.cpp:30
+    // Ignore predicates that are not intended for the assembler.
+    if (!Pred->getValueAsBit("AssemblerMatcherPredicate"))
+      continue;
----------------
Just a thought, shouldn't this string be an argument?
Indeed, if we really want to reuse the functionality we shouldn't bake assembler specific stuff in.

I'm fine with a comment saying that if we want to extend the reusability we would need to pass an additional parameter.


================
Comment at: utils/TableGen/SubtargetFeatureInfo.h:42
+  /// subtarget.
+  static void emitComputeAvailableFeatures(
+      std::string TargetName, std::string ClassName,
----------------
Add comments on what TargetName and ClassName are used.
Also, don't copy the string -> StringRef or const &


https://reviews.llvm.org/D25614