DavidSpickett added inline comments.
================ Comment at: clang/include/clang/Basic/CodeGenOptions.h:385 /// On x86 this can be "fs" or "gs". + /// On AArch64 this can be any value of llvm::AArch64SysReg::SysReg. std::string StackProtectorGuardReg; ---------------- This is now used for TLS on x86 and sysreg on AArch64 so how about: ``` /// The TLS base register when StackProtectorGuard is "tls", or register /// used to store the stack canary for "sysreg". /// On x86 this can be "fs" or "gs". /// On AArch64 this can only be "sp_el0". ``` ================ 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) ---------------- nickdesaulniers wrote: > DavidSpickett wrote: > > 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. > I don't think so; GCC seems to support `tls` for x86 but not for aarch64. > https://godbolt.org/z/6WjEPfhT5 Fair enough, seems unlikely it would work in any case. ================ Comment at: llvm/test/CodeGen/AArch64/stack-guard-sysreg.ll:20 +; RUN: --stack-protector-guard-offset=-1 -o - | \ +; RUN: FileCheck --check-prefix=CHECK --check-prefix=CHECK-NPOT-NEG-OFFSET %s + ---------------- What does NPOT stand for here, something to do with the offset not being a multiple of 8 bytes? 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