Revert "[RISCV] Implement KCFI operand bundle lowering"

This reverts commit 62fa708.

Reverting to investigate -verify-machineinstrs errors in MIR tests.

clang/lib/CodeGen/BackendUtil.cpp

@@ -631,7 +631,7 @@ static void addKCFIPass(const Triple &TargetTriple, const LangOptions &LangOpts,
                         PassBuilder &PB) {
   // If the back-end supports KCFI operand bundle lowering, skip KCFIPass.
   if (TargetTriple.getArch() == llvm::Triple::x86_64 ||
-      TargetTriple.isAArch64(64) || TargetTriple.isRISCV())
+      TargetTriple.isAArch64(64))
     return;
 
   // Ensure we lower KCFI operand bundles with -O0.

llvm/lib/Target/RISCV/RISCVAsmPrinter.cpp

@@ -19,7 +19,6 @@
 #include "RISCVMachineFunctionInfo.h"
 #include "RISCVTargetMachine.h"
 #include "TargetInfo/RISCVTargetInfo.h"
-#include "llvm/ADT/APInt.h"
 #include "llvm/ADT/Statistic.h"
 #include "llvm/BinaryFormat/ELF.h"
 #include "llvm/CodeGen/AsmPrinter.h"
@@ -73,7 +72,6 @@ class RISCVAsmPrinter : public AsmPrinter {
   typedef std::tuple<unsigned, uint32_t> HwasanMemaccessTuple;
   std::map<HwasanMemaccessTuple, MCSymbol *> HwasanMemaccessSymbols;
   void LowerHWASAN_CHECK_MEMACCESS(const MachineInstr &MI);
-  void LowerKCFI_CHECK(const MachineInstr &MI);
   void EmitHwasanMemaccessSymbols(Module &M);
 
   // Wrapper needed for tblgenned pseudo lowering.
@@ -152,9 +150,6 @@ void RISCVAsmPrinter::emitInstruction(const MachineInstr *MI) {
   case RISCV::HWASAN_CHECK_MEMACCESS_SHORTGRANULES:
     LowerHWASAN_CHECK_MEMACCESS(*MI);
     return;
-  case RISCV::KCFI_CHECK:
-    LowerKCFI_CHECK(*MI);
-    return;
   case RISCV::PseudoRVVInitUndefM1:
   case RISCV::PseudoRVVInitUndefM2:
   case RISCV::PseudoRVVInitUndefM4:
@@ -310,92 +305,6 @@ void RISCVAsmPrinter::LowerHWASAN_CHECK_MEMACCESS(const MachineInstr &MI) {
   EmitToStreamer(*OutStreamer, MCInstBuilder(RISCV::PseudoCALL).addExpr(Expr));
 }
 
-void RISCVAsmPrinter::LowerKCFI_CHECK(const MachineInstr &MI) {
-  Register AddrReg = MI.getOperand(0).getReg();
-  assert(std::next(MI.getIterator())->isCall() &&
-         "KCFI_CHECK not followed by a call instruction");
-  assert(std::next(MI.getIterator())->getOperand(0).getReg() == AddrReg &&
-         "KCFI_CHECK call target doesn't match call operand");
-
-  // Temporary registers for comparing the hashes. If a register is used
-  // for the call target, or reserved by the user, we can clobber another
-  // temporary register as the check is immediately followed by the
-  // call. The check defaults to X6/X7, but can fall back to X28-X31 if
-  // needed.
-  unsigned ScratchRegs[] = {RISCV::X6, RISCV::X7};
-  unsigned NextReg = RISCV::X28;
-  auto isRegAvailable = [&](unsigned Reg) {
-    return Reg != AddrReg && !STI->isRegisterReservedByUser(Reg);
-  };
-  for (auto &Reg : ScratchRegs) {
-    if (isRegAvailable(Reg))
-      continue;
-    while (!isRegAvailable(NextReg))
-      ++NextReg;
-    Reg = NextReg++;
-    if (Reg > RISCV::X31)
-      report_fatal_error("Unable to find scratch registers for KCFI_CHECK");
-  }
-
-  if (AddrReg == RISCV::X0) {
-    // Checking X0 makes no sense. Instead of emitting a load, zero
-    // ScratchRegs[0].
-    EmitToStreamer(*OutStreamer, MCInstBuilder(RISCV::ADDI)
-                                     .addReg(ScratchRegs[0])
-                                     .addReg(RISCV::X0)
-                                     .addImm(0));
-  } else {
-    // Adjust the offset for patchable-function-prefix. This assumes that
-    // patchable-function-prefix is the same for all functions.
-    int NopSize = STI->hasStdExtCOrZca() ? 2 : 4;
-    int64_t PrefixNops = 0;
-    (void)MI.getMF()
-        ->getFunction()
-        .getFnAttribute("patchable-function-prefix")
-        .getValueAsString()
-        .getAsInteger(10, PrefixNops);
-
-    // Load the target function type hash.
-    EmitToStreamer(*OutStreamer, MCInstBuilder(RISCV::LW)
-                                     .addReg(ScratchRegs[0])
-                                     .addReg(AddrReg)
-                                     .addImm(-(PrefixNops * NopSize + 4)));
-  }
-
-  // Load the expected 32-bit type hash.
-  const int64_t Type = MI.getOperand(1).getImm();
-  const int64_t Hi20 = ((Type + 0x800) >> 12) & 0xFFFFF;
-  const int64_t Lo12 = SignExtend64<12>(Type);
-  if (Hi20) {
-    EmitToStreamer(
-        *OutStreamer,
-        MCInstBuilder(RISCV::LUI).addReg(ScratchRegs[1]).addImm(Hi20));
-  }
-  if (Lo12 || Hi20 == 0) {
-    EmitToStreamer(*OutStreamer,
-                   MCInstBuilder((STI->hasFeature(RISCV::Feature64Bit) && Hi20)
-                                     ? RISCV::ADDIW
-                                     : RISCV::ADDI)
-                       .addReg(ScratchRegs[1])
-                       .addReg(ScratchRegs[1])
-                       .addImm(Lo12));
-  }
-
-  // Compare the hashes and trap if there's a mismatch.
-  MCSymbol *Pass = OutContext.createTempSymbol();
-  EmitToStreamer(*OutStreamer,
-                 MCInstBuilder(RISCV::BEQ)
-                     .addReg(ScratchRegs[0])
-                     .addReg(ScratchRegs[1])
-                     .addExpr(MCSymbolRefExpr::create(Pass, OutContext)));
-
-  MCSymbol *Trap = OutContext.createTempSymbol();
-  OutStreamer->emitLabel(Trap);
-  EmitToStreamer(*OutStreamer, MCInstBuilder(RISCV::EBREAK));
-  emitKCFITrapEntry(*MI.getMF(), Trap);
-  OutStreamer->emitLabel(Pass);
-}
-
 void RISCVAsmPrinter::EmitHwasanMemaccessSymbols(Module &M) {
   if (HwasanMemaccessSymbols.empty())
     return;

llvm/lib/Target/RISCV/RISCVISelLowering.cpp

@@ -15395,24 +15395,17 @@ SDValue RISCVTargetLowering::LowerCall(CallLoweringInfo &CLI,
   if (Glue.getNode())
     Ops.push_back(Glue);
 
-  assert((!CLI.CFIType || CLI.CB->isIndirectCall()) &&
-         "Unexpected CFI type for a direct call");
-
   // Emit the call.
   SDVTList NodeTys = DAG.getVTList(MVT::Other, MVT::Glue);
 
   if (IsTailCall) {
     MF.getFrameInfo().setHasTailCall();
     SDValue Ret = DAG.getNode(RISCVISD::TAIL, DL, NodeTys, Ops);
-    if (CLI.CFIType)
-      Ret.getNode()->setCFIType(CLI.CFIType->getZExtValue());
     DAG.addNoMergeSiteInfo(Ret.getNode(), CLI.NoMerge);
     return Ret;
   }
 
   Chain = DAG.getNode(RISCVISD::CALL, DL, NodeTys, Ops);
-  if (CLI.CFIType)
-    Chain.getNode()->setCFIType(CLI.CFIType->getZExtValue());
   DAG.addNoMergeSiteInfo(Chain.getNode(), CLI.NoMerge);
   Glue = Chain.getValue(1);
 
@@ -16871,24 +16864,6 @@ bool RISCVTargetLowering::lowerInterleavedStore(StoreInst *SI,
   return true;
 }
 
-MachineInstr *
-RISCVTargetLowering::EmitKCFICheck(MachineBasicBlock &MBB,
-                                   MachineBasicBlock::instr_iterator &MBBI,
-                                   const TargetInstrInfo *TII) const {
-  assert(MBBI->isCall() && MBBI->getCFIType() &&
-         "Invalid call instruction for a KCFI check");
-  assert(is_contained({RISCV::PseudoCALLIndirect, RISCV::PseudoTAILIndirect},
-                      MBBI->getOpcode()));
-
-  MachineOperand &Target = MBBI->getOperand(0);
-  Target.setIsRenamable(false);
-
-  return BuildMI(MBB, MBBI, MBBI->getDebugLoc(), TII->get(RISCV::KCFI_CHECK))
-      .addReg(Target.getReg())
-      .addImm(MBBI->getCFIType())
-      .getInstr();
-}
-
 #define GET_REGISTER_MATCHER
 #include "RISCVGenAsmMatcher.inc"
 

llvm/lib/Target/RISCV/RISCVISelLowering.h

@@ -759,12 +759,6 @@ class RISCVTargetLowering : public TargetLowering {
   bool lowerInterleavedStore(StoreInst *SI, ShuffleVectorInst *SVI,
                              unsigned Factor) const override;
 
-  bool supportKCFIBundles() const override { return true; }
-
-  MachineInstr *EmitKCFICheck(MachineBasicBlock &MBB,
-                              MachineBasicBlock::instr_iterator &MBBI,
-                              const TargetInstrInfo *TII) const override;
-
   /// RISCVCCAssignFn - This target-specific function extends the default
   /// CCValAssign with additional information used to lower RISC-V calling
   /// conventions.

llvm/lib/Target/RISCV/RISCVInstrInfo.cpp

@@ -1265,27 +1265,13 @@ unsigned RISCVInstrInfo::getInstSizeInBytes(const MachineInstr &MI) const {
     }
   }
 
-  if (Opcode == TargetOpcode::BUNDLE)
-    return getInstBundleLength(MI);
-
   if (MI.getParent() && MI.getParent()->getParent()) {
     if (isCompressibleInst(MI, STI))
       return 2;
   }
   return get(Opcode).getSize();
 }
 
-unsigned RISCVInstrInfo::getInstBundleLength(const MachineInstr &MI) const {
-  unsigned Size = 0;
-  MachineBasicBlock::const_instr_iterator I = MI.getIterator();
-  MachineBasicBlock::const_instr_iterator E = MI.getParent()->instr_end();
-  while (++I != E && I->isInsideBundle()) {
-    assert(!I->isBundle() && "No nested bundle!");
-    Size += getInstSizeInBytes(*I);
-  }
-  return Size;
-}
-
 bool RISCVInstrInfo::isAsCheapAsAMove(const MachineInstr &MI) const {
   const unsigned Opcode = MI.getOpcode();
   switch (Opcode) {

llvm/lib/Target/RISCV/RISCVInstrInfo.h

@@ -237,9 +237,6 @@ class RISCVInstrInfo : public RISCVGenInstrInfo {
 
 protected:
   const RISCVSubtarget &STI;
-
-private:
-  unsigned getInstBundleLength(const MachineInstr &MI) const;
 };
 
 namespace RISCV {

llvm/lib/Target/RISCV/RISCVInstrInfo.td

@@ -1887,13 +1887,6 @@ def HWASAN_CHECK_MEMACCESS_SHORTGRANULES
            [(int_hwasan_check_memaccess_shortgranules X5, GPRJALR:$ptr,
                                                       (i32 timm:$accessinfo))]>;
 
-// This gets lowered into a 20-byte instruction sequence (at most)
-let hasSideEffects = 0, mayLoad = 1, mayStore = 0,
-    Defs = [ X6, X7, X28, X29, X30, X31 ], Size = 20 in {
-def KCFI_CHECK
-  : Pseudo<(outs), (ins GPRJALR:$ptr, i32imm:$type), []>, Sched<[]>;
-}
-
 /// Simple optimization
 def : Pat<(XLenVT (add GPR:$rs1, (AddiPair:$rs2))),
           (ADDI (ADDI GPR:$rs1, (AddiPairImmLarge AddiPair:$rs2)),

llvm/lib/Target/RISCV/RISCVTargetMachine.cpp

@@ -76,7 +76,6 @@ extern "C" LLVM_EXTERNAL_VISIBILITY void LLVMInitializeRISCVTarget() {
   RegisterTargetMachine<RISCVTargetMachine> Y(getTheRISCV64Target());
   auto *PR = PassRegistry::getPassRegistry();
   initializeGlobalISel(*PR);
-  initializeKCFIPass(*PR);
   initializeRISCVMakeCompressibleOptPass(*PR);
   initializeRISCVGatherScatterLoweringPass(*PR);
   initializeRISCVCodeGenPreparePass(*PR);
@@ -334,10 +333,7 @@ bool RISCVPassConfig::addGlobalInstructionSelect() {
   return false;
 }
 
-void RISCVPassConfig::addPreSched2() {
-  // Emit KCFI checks for indirect calls.
-  addPass(createKCFIPass());
-}
+void RISCVPassConfig::addPreSched2() {}
 
 void RISCVPassConfig::addPreEmitPass() {
   addPass(&BranchRelaxationPassID);
@@ -361,11 +357,6 @@ void RISCVPassConfig::addPreEmitPass2() {
   // possibility for other passes to break the requirements for forward
   // progress in the LR/SC block.
   addPass(createRISCVExpandAtomicPseudoPass());
-
-  // KCFI indirect call checks are lowered to a bundle.
-  addPass(createUnpackMachineBundles([&](const MachineFunction &MF) {
-    return MF.getFunction().getParent()->getModuleFlag("kcfi");
-  }));
 }
 
 void RISCVPassConfig::addMachineSSAOptimization() {

llvm/test/CodeGen/RISCV/O0-pipeline.ll

@@ -51,7 +51,6 @@
 ; CHECK-NEXT:       Machine Optimization Remark Emitter
 ; CHECK-NEXT:       Prologue/Epilogue Insertion & Frame Finalization
 ; CHECK-NEXT:       Post-RA pseudo instruction expansion pass
-; CHECK-NEXT:       Insert KCFI indirect call checks
 ; CHECK-NEXT:       Analyze Machine Code For Garbage Collection
 ; CHECK-NEXT:       Insert fentry calls
 ; CHECK-NEXT:       Insert XRay ops
@@ -67,7 +66,6 @@
 ; CHECK-NEXT:       Stack Frame Layout Analysis
 ; CHECK-NEXT:       RISC-V pseudo instruction expansion pass
 ; CHECK-NEXT:       RISC-V atomic pseudo instruction expansion pass
-; CHECK-NEXT:       Unpack machine instruction bundles
 ; CHECK-NEXT:       Lazy Machine Block Frequency Analysis
 ; CHECK-NEXT:       Machine Optimization Remark Emitter
 ; CHECK-NEXT:       RISC-V Assembly Printer

llvm/test/CodeGen/RISCV/O3-pipeline.ll

@@ -155,7 +155,6 @@
 ; CHECK-NEXT:       Tail Duplication
 ; CHECK-NEXT:       Machine Copy Propagation Pass
 ; CHECK-NEXT:       Post-RA pseudo instruction expansion pass
-; CHECK-NEXT:       Insert KCFI indirect call checks
 ; CHECK-NEXT:       MachineDominator Tree Construction
 ; CHECK-NEXT:       Machine Natural Loop Construction
 ; CHECK-NEXT:       Post RA top-down list latency scheduler
@@ -181,7 +180,6 @@
 ; CHECK-NEXT:       RISC-V Zcmp move merging pass
 ; CHECK-NEXT:       RISC-V pseudo instruction expansion pass
 ; CHECK-NEXT:       RISC-V atomic pseudo instruction expansion pass
-; CHECK-NEXT:       Unpack machine instruction bundles
 ; CHECK-NEXT:       Lazy Machine Block Frequency Analysis
 ; CHECK-NEXT:       Machine Optimization Remark Emitter
 ; CHECK-NEXT:       RISC-V Assembly Printer