[PATCH] D134353: [AArch64] Add all predecessor archs in target info

[Dave Green via Phabricator via llvm-commits]

dmgreen added a comment.

I think the idea makes sense. I was expecting some tests to change, where they were previously checking for "..,+v8.2a" and would now see v8.1a and v8a, etc.



================
Comment at: clang/lib/Basic/Targets/AArch64.cpp:531
+
+  llvm::AArch64::ArchKind AK = llvm::AArch64::getCPUArchKind(Name);
+  if ("9" == getArchVersionString(AK)) {
----------------
Why getCPUArchKind?


================
Comment at: clang/lib/Driver/ToolChains/Arch/AArch64.cpp:420
     // Register the iterator position if this is an architecture feature
     if (ArchFeatPos == -1 && (V8Version != -1 || V9Version != -1))
       ArchFeatPos = I - Features.begin();
----------------
This looks like it is expecting to add new features just after the arch feature, which may not be the correct one. I'm not sure how much that matters but I guess the idea is that those features can later be disabled.


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

https://reviews.llvm.org/D134353