[PATCH] D138792: [AArch64] Improve TargetParser API

[Tomas Matheson via Phabricator via cfe-commits]

tmatheson added inline comments.


================
Comment at: clang/lib/Basic/Targets/AArch64.cpp:532
     getTargetDefinesARMV81A(Opts, Builder);
-    break;
-  case llvm::AArch64::ArchKind::ARMV8_2A:
+  if (*ArchInfo == llvm::AArch64::ARMV8_2A)
     getTargetDefinesARMV82A(Opts, Builder);
----------------
danielkiss wrote:
> 
I'll fix this when I push


================
Comment at: clang/lib/Driver/ToolChains/Arch/AArch64.cpp:172
+       *ArchInfo == llvm::AArch64::ARMV9_1A ||
+       *ArchInfo == llvm::AArch64::ARMV9_2A)) {
     Features.push_back("+sve");
----------------
danielkiss wrote:
> Would be nice to add a custom operator to `ArchInfo` to say `*ArchInfo >= llvm::AArch64::ARMV9A`
> because it looks to me here the `llvm::AArch64::ARMV9_3A` and `llvm::AArch64::ARMV9_4A` are missing.
Good catch. This could be written as `if(ArchInfo.implies(ARMV9A))`, but I'll leave that for a follow up patch. I opted against a custom operator because they generally make things less understandable, except in cases where the ordering is very obvious, e.g. numeric types. For example 9.2 does not imply 8.8. If you want to do an actual numerical comparison of version numbers you can compare ArchInfo.Versions directly.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138792