DavidSpickett added inline comments.
================ Comment at: clang/lib/Driver/ToolChains/Clang.cpp:3110 + } + if (EffectiveTriple.isAArch64() && Value != "sysreg" && Value != "global") { + D.Diag(diag::err_drv_invalid_value_with_suggestion) ---------------- Shouldn't this also allow "tls"? At least that's what the previous code works out to, I don't know if that actually works on AArch64 or if it just didn't error. ================ Comment at: clang/lib/Driver/ToolChains/Clang.cpp:3138 } + if (EffectiveTriple.isAArch64() && Value != "sp_el0") { + D.Diag(diag::err_drv_invalid_value_with_suggestion) ---------------- nickdesaulniers wrote: > nickdesaulniers wrote: > > nickdesaulniers wrote: > > > nickdesaulniers wrote: > > > > nickdesaulniers wrote: > > > > > TODO: can we re-use `AArch64SysReg::lookupSysRegByName` in the > > > > > frontend? > > > > I don't think so because > > > > `llvm/lib/Target/AArch64/Utils/AArch64BaseInfo.h` is under lib/ not > > > > include/. Not sure if I should just remove reg validation? > > > Guidance provided by @echristo and @jyknight was that we should avoid > > > such linkage requirements on Target/, so instead I'll work on adding a > > > helper to clang/lib/Driver/ToolChains/Arch/AArch64.cpp that duplicates > > > logic from `AArch64SysReg::lookupSysRegByName`. > > It looks like there's ~1000 possible sysregs for aarch64 ATM; do we really > > want to add all of those to clang? > I'm going to post that as a separate commit/review on top of this series, > that way it doesn't pollute this code review. This is ready to be reviewed. If the number of different registers people actually use with this option is somewhere < 10 I'd just hardcode the names here as needed. (a large amount of those sysregs won't be suitable for this purpose anyway) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D100919/new/ https://reviews.llvm.org/D100919 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits