[PATCH] D113776: [Clang][SVE] Properly enable/disable dependant SVE target features based upon +(no)sve.* options

[Sander de Smalen via Phabricator via llvm-commits]

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

LGTM!



================
Comment at: clang/lib/Driver/ToolChains/Arch/AArch64.cpp:82-88
+    if (Feature == "sve2")
+      Features.push_back("+sve");
+    else if (Feature == "sve2-bitperm" || Feature == "sve2-sha3" ||
+             Feature == "sve2-aes" || Feature == "sve2-sm4") {
+      Features.push_back("+sve");
+      Features.push_back("+sve2");
+    } else if (Feature == "nosve") {
----------------
bsmith wrote:
> sdesmalen wrote:
> > ^^^ Are the above changes necessary? i.e. if only `+sve2-sha3` is set as target feature, I thought LLVM automatically infers that it requires `+sve` and `+sve2`. Is that not the case?
> That is the case yes, however this code is to catch combinations, for example `+sve2-bitperm+nosve2` needs to keep `sve` enabled, if +sve was never in the feature set this wouldn't be the case.
That makes sense, thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113776