[PATCH] D120906: [ARM][AArch64] generate subtarget feature flags [NFC]

[Fangrui Song via Phabricator via llvm-commits]

MaskRay accepted this revision.
MaskRay added a comment.
This revision is now accepted and ready to land.

Looks great!



================
Comment at: llvm/lib/Target/AArch64/AArch64Subtarget.h:158
+// Getters for SubtargetFeatures defined in tablegen
+#define GET_SUBTARGETINFO_MACRO(NAME, ATTRIBUTE, DEFAULT, GETTER)              \
+  bool GETTER() const { return ATTRIBUTE; }
----------------
Is `NAME` used? Or do you expect it may be used in the future?


================
Comment at: llvm/lib/Target/ARM/ARM.td:53
 
+// If true, the floating point unit supports double precision.
 def FeatureFP64           : SubtargetFeature<"fp64", "HasFP64", "true",
----------------
It seems that you just copy comments from `*Subtarget.h` to the TableGen file. Seems useful to push the comment in a separate commit to make it clear this patch changes TableGen very little.


================
Comment at: llvm/lib/Target/ARM/ARM.td:177
+
+// If true, processor supports TrustZone security extensions
 def FeatureTrustZone      : SubtargetFeature<"trustzone", "HasTrustZone", "true",
----------------
Previous comments use `True if` while this one uses `If true,`. Try improving consistency.

Also, end comments with a period.


================
Comment at: llvm/utils/TableGen/SubtargetEmitter.cpp:1811
+  for (const Record *Feature : FeatureList) {
+
+    const auto Name = Feature->getValueAsString("Name");
----------------
delete blank line after `for`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120906