serge-sans-paille marked 2 inline comments as done. serge-sans-paille added inline comments.
================ Comment at: clang/lib/Driver/ToolChains/Clang.cpp:2910 + + if (Arg *A = Args.getLastArg(options::OPT_fnostack_clash_protection, + options::OPT_fstack_clash_protection)) { ---------------- craig.topper wrote: > 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' I wasn't aware of that API, thanks for pointing this out o/ ================ Comment at: llvm/lib/Target/X86/X86ISelLowering.cpp:31080 + + BuildMI(blockMBB, DL, TII->get(IsLP64 ? X86::SUB64ri32 : X86::SUB32ri), + physSPReg) ---------------- craig.topper wrote: > This uses physSPReg, but doesn't match the condition for when physSPReg is a > 64-bit register. Same at several places below. Sorry, I don't understand your remark. Can you elaborate? ================ Comment at: llvm/lib/Target/X86/X86InstrCompiler.td:130 + (X86ProbedAlloca GR64:$size))]>, + Requires<[In64BitMode]>; } ---------------- craig.topper wrote: > Why is this In64BitMode, but above is NotLP64. Shouldn't they be opposites? > Looks like this was just copied from SEG_ALLOCA above? Yeah, these intrinsics are just the same as the one above, except they have a different name. And they get lowered differently. I'm not familiar with that part of LLVM, can you suggest a better approach? 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