nickdesaulniers added inline comments.

================
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:
> > > > 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.


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

Reply via email to