[PATCH] D148385: [RISCV] Implement KCFI operand bundle lowering

This revision was automatically updated to reflect the committed changes. Closed by commit rG62fa708ceb02: [RISCV] Implement KCFI operand bundle lowering (authored by samitolvanen). Herald added a subscriber: wangpc. Changed prior to commit: https://reviews.llvm.org/D148385?vs=531476&id=534032#

[PATCH] D148385: [RISCV] Implement KCFI operand bundle lowering

MaskRay accepted this revision. MaskRay added a comment. This revision is now accepted and ready to land. Will be good to wait a few days to give regular RISC-V backend developers a chance to leave any final comments. Comment at: llvm/lib/Target/RISCV/RISCVAsmPrinter.cpp:332 +

[PATCH] D148385: [RISCV] Implement KCFI operand bundle lowering

samitolvanen added a comment. > Needs some thoughts to merge the above paragraphs with the existing one Great points. I updated the commit message and tried to capture most of these, PTAL. Comment at: llvm/lib/Target/RISCV/RISCVAsmPrinter.cpp:332 + continue; +while (

[PATCH] D148385: [RISCV] Implement KCFI operand bundle lowering

samitolvanen updated this revision to Diff 531476. samitolvanen marked 9 inline comments as done. samitolvanen added a comment. Addressed feedback. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148385/new/ https://reviews.llvm.org/D148385 Files:

[PATCH] D148385: [RISCV] Implement KCFI operand bundle lowering

MaskRay added inline comments. Comment at: llvm/lib/Target/RISCV/RISCVISelLowering.cpp:14427 SDValue Callee = CLI.Callee; + bool IsCFICall = CLI.CB && CLI.CB->isIndirectCall() && CLI.CFIType; bool &IsTailCall = CLI.IsTailCall; If `CLI.CFIType != 0`, is `CL

[PATCH] D148385: [RISCV] Implement KCFI operand bundle lowering

MaskRay added a comment. Thanks for the description. The code generally looks good. If the kernel wants a specific code sequence, implementing conditions in the LLVMCodeGen looks good to me. Consider adding a link to the discussion. I wonder whether it will be more useful to give more context t

[PATCH] D148385: [RISCV] Implement KCFI operand bundle lowering

samitolvanen added a comment. In D148385#4397995 , @MaskRay wrote: > `KCFI_CHECK` lowering has some complexity to allocate a temporary register. > This needs to follow the calling convention which can be modified by many > compiler options and function

[PATCH] D148385: [RISCV] Implement KCFI operand bundle lowering

MaskRay added a comment. `KCFI_CHECK` lowering has some complexity to allocate a temporary register. This needs to following the calling convention which can be modified by many compiler options and function attributes. I wonder whether we can move the if-condition part of the expanded code se

[PATCH] D148385: [RISCV] Implement KCFI operand bundle lowering

samitolvanen updated this revision to Diff 520795. samitolvanen added a comment. Dropped the arch-specific KCFI machine function pass. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148385/new/ https://reviews.llvm.org/D148385 Files: clang/lib/Co

[PATCH] D148385: [RISCV] Implement KCFI operand bundle lowering

samitolvanen planned changes to this revision. samitolvanen added a comment. Uploaded D149915 to remove the arch-specific KCFI passes. I'll update this patch once we clean that up. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://revi

[PATCH] D148385: [RISCV] Implement KCFI operand bundle lowering for RV64

samitolvanen updated this revision to Diff 514708. samitolvanen added a comment. Addressed most of the feedback. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148385/new/ https://reviews.llvm.org/D148385 Files: clang/lib/CodeGen/BackendUtil.cpp

[PATCH] D148385: [RISCV] Implement KCFI operand bundle lowering for RV64

samitolvanen marked 9 inline comments as done. samitolvanen added a comment. In D148385#4275617 , @jrtc27 wrote: > I can't help but feel the core of this should be target-independent, with TLI > or similar hooks to actually do the target-specific bits?

[PATCH] D148385: [RISCV] Implement KCFI operand bundle lowering for RV64

craig.topper added inline comments. Comment at: llvm/utils/gn/secondary/llvm/lib/Target/RISCV/BUILD.gn:81 "RISCVInstrInfo.cpp", +"RISCVKCFI.cpp", "RISCVMCInstLower.cpp", Isn't this done automatically by the GN Syncbot when CMakeLists.txt is updated?

[PATCH] D148385: [RISCV] Implement KCFI operand bundle lowering for RV64

craig.topper added inline comments. Comment at: llvm/lib/Target/RISCV/RISCVAsmPrinter.cpp:310 +EmitToStreamer(*OutStreamer, MCInstBuilder(RISCV::ADDI) + .addReg(ScratchRegs[0]) + .addReg(RISCV::X0) --

[PATCH] D148385: [RISCV] Implement KCFI operand bundle lowering for RV64

craig.topper added inline comments. Comment at: llvm/lib/Target/RISCV/RISCVAsmPrinter.cpp:310 +EmitToStreamer(*OutStreamer, MCInstBuilder(RISCV::ADDI) + .addReg(ScratchRegs[0]) + .addReg(RISCV::X0) --

[PATCH] D148385: [RISCV] Implement KCFI operand bundle lowering for RV64

jrtc27 added inline comments. Comment at: llvm/lib/Target/RISCV/RISCVAsmPrinter.cpp:277 +void RISCVAsmPrinter::LowerKCFI_CHECK(const MachineInstr &MI) { + assert(STI->is64Bit() && "KCFI_CHECK requires RV64"); + What about this code relies on it being 64-bit? =

[PATCH] D148385: [RISCV] Implement KCFI operand bundle lowering for RV64

jrtc27 added a comment. I can't help but feel the core of this should be target-independent, with TLI or similar hooks to actually do the target-specific bits? Comment at: llvm/test/CodeGen/RISCV/kcfi.ll:1 +; RUN: llc -mtriple=riscv64 -verify-machineinstrs -riscv-no-aliases <

[PATCH] D148385: [RISCV] Implement KCFI operand bundle lowering for RV64

nickdesaulniers added inline comments. Comment at: llvm/lib/Target/RISCV/RISCVAsmPrinter.cpp:299-300 + ++NextReg; +Reg = NextReg++; +if (Reg > RISCV::X31) + report_fatal_error("Unable to find scratch registers for KCFI_CHECK"); if (++NextReg > R

[PATCH] D148385: [RISCV] Implement KCFI operand bundle lowering for RV64

nickdesaulniers added inline comments. Comment at: llvm/lib/Target/RISCV/RISCVKCFI.cpp:106-107 + for (MachineBasicBlock &MBB : MF) { +for (MachineBasicBlock::instr_iterator MII = MBB.instr_begin(), + MIE = MBB.instr_end(); +

[PATCH] D148385: [RISCV] Implement KCFI operand bundle lowering for RV64

samitolvanen created this revision. Herald added subscribers: jobnoorman, luke, VincentWu, ormris, vkmr, frasercrmck, evandro, luismarques, apazos, sameer.abuasal, pengfei, s.egerton, Jim, benna, psnobl, jocewei, PkmX, the_o, brucehoult, MartinMosbeck, rogfer01, edward-jones, zzheng, jrtc27, shi