[PATCH] D120257: [AArch64][SME] Replace +streaming-sve feature with +streaming-compatible-<feature>.

Tue Feb 22 05:02:50 PST 2022

paulwalker-arm added inline comments.

================
Comment at: llvm/lib/Target/AArch64/AArch64.td:441-460
+def FeatureStreamingCompatibleNEON
+    : SubtargetFeature<"streaming-compatible-neon",
+                       "HasStreamingCompatibleNEON", "true",
+                       "Enable subset of NEON instructions that are valid in "
+                       "both streaming SVE mode as well as regular mode",
+                       [FeatureFPARMv8, FeatureFullFP16]>;
+
----------------
Do we need to split the feature flags based on other feature flag boundaries?  Why not just `FeatureStreamingCompatible`?  I'm not saying the above is wrong, but it does seem like an extra level of indirection so just trying to understand the reasoning.

================
Comment at: llvm/lib/Target/AArch64/AArch64Subtarget.h:136
   // SVE extensions
+  bool HasStreamingCompatibleSVE = false;
   bool HasSVE = false;
----------------
Up to you but stylistically I think it would be better if all the SME related flags live together.  I understand this flag relates to SVE but if it wasn't for SME it wouldn't exist.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120257