[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