[PATCH] D91773: [AArch64] Add SubtargetFeatures for v8.7-A options

[Oliver Stannard (Linaro) via Phabricator via llvm-commits]

ostannard added inline comments.


================
Comment at: llvm/lib/Target/AArch64/AArch64.td:406
+
+def FeatureHCX : SubtargetFeature<
+    "hcx", "HasHCX", "true", "Enable Armv8.7-A HCRX_EL2 system register">;
----------------
Why is this `FeatureHCX`, and not `FeatureHCRX` for consistency with the register name?


================
Comment at: llvm/lib/Target/AArch64/AsmParser/AArch64AsmParser.cpp:2928
     Str += "ARMv8.6a";
-  else if (FBS[AArch64::HasV8_7aOps])
+  else if (FBS[AArch64::HasV8_7aOps] || FBS[AArch64::FeatureXS])
     Str += "ARMv8.7a";
----------------
Why does this need a special case? Would it not be better to add these extensions to `ExtensionMap` above?


================
Comment at: llvm/lib/Target/AArch64/AsmParser/AArch64AsmParser.cpp:3019
         TLBIorig->NeedsReg,
-        TLBIorig->FeaturesRequired);
+        HasnXSQualifier
+            ? TLBIorig->FeaturesRequired | FeatureBitset({AArch64::FeatureXS})
----------------
Could we represent these operand variants in the tablegen, to avoid needing to special-case them here. We could make the TLBI tablegen class a multiclass which expands to the regular and nXS variants, with the appropriate feature requirements set.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91773