[PATCH] D128029: [AArch64] Add target feature "all"

[Stephen Hines via Phabricator via llvm-commits]

srhines added inline comments.


================
Comment at: llvm/lib/Target/AArch64/AArch64.td:558-566
 
+// Only available to disassemblers.
+def FeatureAll
+    : SubtargetFeature<"all", "IsAll", "true", "Enable all instructions", [
+      HasV9_3aOps, FeatureCrypto, FeatureETE, FeatureFP16FML, FeatureMTE,
+      FeatureMatMulFP32, FeatureMatMulFP64, FeaturePerfMon, FeatureRandGen,
+      FeatureRME, FeatureSMEF64, FeatureSMEI64, FeatureSPE, FeatureSVE2AES,
----------------
MaskRay wrote:
> nickdesaulniers wrote:
> > c-rhodes wrote:
> > > I think this will introduce a bit of a maintenance burden and I can see new features being missed, could this be generated by tablegen?
> > Or perhaps an assertion somewhere if the possible feature list grows to alert this needs to be updated?
> Explained in a main comment:
> 
> > but lacking some stuff when new target features emerge does not matter: people running llvm-objdump may notice and we can add new target features to "all".
> 
> This controversy is the main point that we do not add a user-facing (e.g. `clang -march`) support for `all`.
I'm inclined to believe that this is a reasonable maintenance burden, but perhaps someone from ARM should weigh in here. It's in precisely the best place to be noticed when adding a new AArch64 subtarget/feature. Ultimately this is being added to support improved experiences for llvm-objdump, and it does that. If there are missing subtargets/features in the future, it seems relatively straightforward to add them here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128029