craig.topper added inline comments.
================ Comment at: clang/include/clang/Basic/DiagnosticCommonKinds.td:243 + + def warn_fe_stack_clash_protection_inline_asm : Warning< + "Unable to protect inline asm that clobbers stack pointer against stack clash">; ---------------- Remove "fe_" from this? Looks like thats only there because this used to be DiagnosticFrontendKinds? ================ Comment at: clang/lib/Basic/Targets/X86.h:169 + const char *getSPRegName() const override { + if (getTriple().getArch() == llvm::Triple::x86) ---------------- This probably doesn't work correctly with X32 which is 64-bit but uses 32-bit pointers. Maybe we change this to "bool isSPReg(StringRef Reg)" and pass the register name to it so you can just check both strings always. ================ Comment at: clang/lib/Driver/ToolChains/Clang.cpp:2910 + + if (Arg *A = Args.getLastArg(options::OPT_fnostack_clash_protection, + options::OPT_fstack_clash_protection)) { ---------------- I think this can use Args.hasFlag(options::OPT_fstack_clash_protection, options::OPT_fnostack_clash_protection, false) and then you don't need the 'if' ================ Comment at: llvm/lib/Target/X86/X86ISelLowering.cpp:31080 + + BuildMI(blockMBB, DL, TII->get(IsLP64 ? X86::SUB64ri32 : X86::SUB32ri), + physSPReg) ---------------- This uses physSPReg, but doesn't match the condition for when physSPReg is a 64-bit register. Same at several places below. ================ Comment at: llvm/lib/Target/X86/X86InstrCompiler.td:130 + (X86ProbedAlloca GR64:$size))]>, + Requires<[In64BitMode]>; } ---------------- Why is this In64BitMode, but above is NotLP64. Shouldn't they be opposites? Looks like this was just copied from SEG_ALLOCA above? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68720/new/ https://reviews.llvm.org/D68720 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits