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

[David Spickett via Phabricator via llvm-commits]

DavidSpickett accepted this revision.
DavidSpickett added a comment.

My questions were answered, LGTM.



================
Comment at: clang/lib/Basic/Targets/AArch64.cpp:541
+       --i)
+    Features[llvm::AArch64::getSubArch(i)] = Enabled;
+}
----------------
danielkiss wrote:
> DavidSpickett wrote:
> > It's not clear to me that you need 2 loops or the conditional here. What prevents you from just having:
> > ```
> > llvm::AArch64::ArchKind AK = llvm::AArch64::getSubArchArchKind(Name);
> > for (llvm::AArch64::ArchKind i = llvm::AArch64::convertV9toV8(AK);
> >   i != llvm::AArch64::ArchKind::INVALID; --i)
> >     Features[llvm::AArch64::getSubArch(i)] = Enabled;
> > ```
> > 
> > Given that converting anything that isn't v9 returns itself, then you walk backwards to all the previous architectures. Seems like you only need to do that once but tell me what I'm missing.
> The iterator only works inside the architecture family 9/8.
> so the first loop will set only the v8 counterparts while the second the v9.
Got it, I missed that your operator-- when applied to v9.0 will return invalid. I thought it would go to whatever last v8 there was.


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

https://reviews.llvm.org/D134353