fixup: Address comments

It was not very obvious that bonding and pairing are two different
things, so it is improved now. In addition, mentioned the AArch64
optimization and similarity with it.

Author: djtodoro

llvm/lib/Target/RISCV/RISCVLoadStoreOptimizer.cpp

@@ -6,8 +6,25 @@
 //
 //===----------------------------------------------------------------------===//
 //
-// Bundle loads and stores that operate on consecutive memory locations to take
-// the advantage of hardware load/store bonding.
+// This pass implements two key optimizations for RISC-V memory accesses:
+//   1. Load/Store Pairing: It identifies pairs of load or store instructions
+//      operating on consecutive memory locations and merges them into a single
+//      paired instruction, taking advantage of hardware support for paired
+//      accesses. Much of the pairing logic is adapted from the
+//      AArch64LoadStoreOpt pass.
+//   2. Load/Store Bonding: When direct pairing cannot be applied, the pass
+//   bonds related memory instructions together into a bundle. This preserves
+//   their proximity and prevents reordering that might violate memory
+//   semantics. This technique benefits certain targets (e.g. MIPS P8700) by
+//   ensuring that paired or bonded memory operations remain contiguous.
+//
+// NOTE: The AArch64LoadStoreOpt pass performs additional optimizations such as
+//       merging zero store instructions, promoting loads that read directly
+//       from a preceding store, and merging base register updates with
+//       load/store instructions (via pre-/post-indexed addressing). These
+//       advanced transformations are not yet implemented in the RISC-V pass but
+//       represent potential future enhancements, as similar benefits could be
+//       achieved on RISC-V architectures.
 //
 //===----------------------------------------------------------------------===//
 
@@ -22,7 +39,7 @@
 using namespace llvm;
 
 #define DEBUG_TYPE "riscv-load-store-opt"
-#define RISCV_LOAD_STORE_OPT_NAME "RISCV Load / Store Optimizer"
+#define RISCV_LOAD_STORE_OPT_NAME "RISC-V Load / Store Optimizer"
 namespace {
 
 struct RISCVLoadStoreOpt : public MachineFunctionPass {
@@ -66,6 +83,8 @@ struct RISCVLoadStoreOpt : public MachineFunctionPass {
   const RISCVInstrInfo *TII;
   const RISCVRegisterInfo *TRI;
   LiveRegUnits ModifiedRegUnits, UsedRegUnits;
+  bool EnableLoadStorePairs = false;
+  bool EnableLoadStoreBonding = false;
 };
 } // end anonymous namespace
 
@@ -77,7 +96,9 @@ bool RISCVLoadStoreOpt::runOnMachineFunction(MachineFunction &Fn) {
   if (skipFunction(Fn.getFunction()))
     return false;
   const RISCVSubtarget &Subtarget = Fn.getSubtarget<RISCVSubtarget>();
-  if (!Subtarget.useLoadStorePairs())
+  EnableLoadStorePairs = Subtarget.useLoadStorePairs();
+  EnableLoadStoreBonding = Subtarget.useMIPSLoadStoreBonding();
+  if (!EnableLoadStorePairs && !EnableLoadStoreBonding)
     return false;
 
   bool MadeChange = false;
@@ -107,12 +128,16 @@ bool RISCVLoadStoreOpt::runOnMachineFunction(MachineFunction &Fn) {
 // instruction.
 bool RISCVLoadStoreOpt::tryToPairLdStInst(MachineBasicBlock::iterator &MBBI) {
   MachineInstr &MI = *MBBI;
-  MachineBasicBlock::iterator E = MI.getParent()->end();
+
+  // If this is volatile, it is not a candidate.
+  if (MI.hasOrderedMemoryRef())
+    return false;
 
   if (!TII->isLdStSafeToPair(MI, TRI))
     return false;
 
   // Look ahead for a pairable instruction.
+  MachineBasicBlock::iterator E = MI.getParent()->end();
   bool MergeForward;
   MachineBasicBlock::iterator Paired = findMatchingInsn(MBBI, MergeForward);
   if (Paired != E) {
@@ -130,16 +155,16 @@ bool RISCVLoadStoreOpt::tryConvertToLdStPair(
   default:
     return false;
   case RISCV::SW:
-    PairOpc = RISCV::SWP;
+    PairOpc = RISCV::MIPS_SWP;
     break;
   case RISCV::LW:
-    PairOpc = RISCV::LWP;
+    PairOpc = RISCV::MIPS_LWP;
     break;
   case RISCV::SD:
-    PairOpc = RISCV::SDP;
+    PairOpc = RISCV::MIPS_SDP;
     break;
   case RISCV::LD:
-    PairOpc = RISCV::LDP;
+    PairOpc = RISCV::MIPS_LDP;
     break;
   }
 
@@ -201,8 +226,8 @@ RISCVLoadStoreOpt::findMatchingInsn(MachineBasicBlock::iterator I,
   bool MayLoad = FirstMI.mayLoad();
   Register Reg = FirstMI.getOperand(0).getReg();
   Register BaseReg = FirstMI.getOperand(1).getReg();
-  int Offset = FirstMI.getOperand(2).getImm();
-  int OffsetStride = (*FirstMI.memoperands_begin())->getSize().getValue();
+  int64_t Offset = FirstMI.getOperand(2).getImm();
+  int64_t OffsetStride = (*FirstMI.memoperands_begin())->getSize().getValue();
 
   MergeForward = false;
 
@@ -226,10 +251,9 @@ RISCVLoadStoreOpt::findMatchingInsn(MachineBasicBlock::iterator I,
     if (MI.getOpcode() == FirstMI.getOpcode() &&
         TII->isLdStSafeToPair(MI, TRI)) {
       Register MIBaseReg = MI.getOperand(1).getReg();
-      int MIOffset = MI.getOperand(2).getImm();
+      int64_t MIOffset = MI.getOperand(2).getImm();
 
       if (BaseReg == MIBaseReg) {
-
         if ((Offset != MIOffset + OffsetStride) &&
             (Offset + OffsetStride != MIOffset)) {
           LiveRegUnits::accumulateUsedDefed(MI, ModifiedRegUnits, UsedRegUnits,
@@ -349,12 +373,21 @@ RISCVLoadStoreOpt::mergePairedInsns(MachineBasicBlock::iterator I,
     First = InsertionPoint;
   }
 
-  if (!tryConvertToLdStPair(First, Second))
+  // It may pair them or creaate bundles. The instructions may still be bundled
+  // together, preserving their proximity and the intent of keeping related
+  // memory accesses together. This bundling can help subsequent passes maintain
+  // any implicit ordering or avoid reordering that might violate memory
+  // semantics. For exmaple, MIPS P8700 benefits from it.
+  if (EnableLoadStorePairs && tryConvertToLdStPair(First, Second)) {
+    LLVM_DEBUG(dbgs() << "Pairing load/store:\n    ");
+    LLVM_DEBUG(prev_nodbg(NextI, MBB.begin())->print(dbgs()));
+  } else if (EnableLoadStoreBonding) {
     finalizeBundle(MBB, First.getInstrIterator(),
                    std::next(Second).getInstrIterator());
+    LLVM_DEBUG(dbgs() << "Bonding load/store:\n    ");
+    LLVM_DEBUG(prev_nodbg(NextI, MBB.begin())->print(dbgs()));
+  }
 
-  LLVM_DEBUG(dbgs() << "Bonding pair load/store:\n    ");
-  LLVM_DEBUG(prev_nodbg(NextI, MBB.begin())->print(dbgs()));
   return NextI;
 }
 

llvm/lib/Target/RISCV/RISCVSubtarget.cpp

@@ -62,10 +62,15 @@ static cl::opt<unsigned> RISCVMinimumJumpTableEntries(
     "riscv-min-jump-table-entries", cl::Hidden,
     cl::desc("Set minimum number of entries to use a jump table on RISCV"));
 
+static cl::opt<bool> UseMIPSLoadStorePairsOpt(
+    "mips-riscv-load-store-pairs",
+    cl::desc("RISCV: Enable the load/store pair optimization pass"),
+    cl::init(false), cl::Hidden);
+
 static cl::opt<bool>
-    UseMIPSLoadStorePairsOpt("mips-riscv-load-store-pairs",
+    UseMIPSLoadStoreBondingOpt("mips-riscv-load-store-bonding",
                              cl::desc("RISCV: Optimize for load-store bonding"),
-                             cl::init(false), cl::Hidden);
+                             cl::init(true), cl::Hidden);
 
 static cl::opt<bool>
     UseCCMovInsn("riscv-ccmov", cl::desc("RISCV: Use 'mips.ccmov' instruction"),
@@ -252,6 +257,10 @@ bool RISCVSubtarget::useLoadStorePairs() const {
   return UseMIPSLoadStorePairsOpt && HasVendorXMIPSLSP;
 }
 
+bool RISCVSubtarget::useMIPSLoadStoreBonding() const {
+  return UseMIPSLoadStoreBondingOpt && HasVendorXMIPSLSP;
+}
+
 bool RISCVSubtarget::useCCMovInsn() const {
   return UseCCMovInsn && HasVendorXMIPSCMove;
 }

llvm/lib/Target/RISCV/RISCVSubtarget.h

@@ -189,6 +189,7 @@ class RISCVSubtarget : public RISCVGenSubtargetInfo {
     return is64Bit() ? 64 : 32;
   }
   bool useLoadStorePairs() const;
+  bool useMIPSLoadStoreBonding() const;
   bool useCCMovInsn() const;
   unsigned getFLen() const {
     if (HasStdExtD)

llvm/lib/Target/RISCV/RISCVTargetMachine.cpp

@@ -312,11 +312,12 @@ ScheduleDAGInstrs *
 RISCVTargetMachine::createPostMachineScheduler(MachineSchedContext *C) const {
   ScheduleDAGMI *DAG = nullptr;
   const RISCVSubtarget &ST = C->MF->getSubtarget<RISCVSubtarget>();
-  bool EnableLoadStoreClusteringForLoadStorePairOpt =
-      !ST.getMacroFusions().empty() && ST.useLoadStorePairs();
+  bool EnableLoadStoreClusteringForLoadStoreOpt =
+      !ST.getMacroFusions().empty() &&
+      (ST.useLoadStorePairs() || ST.useMIPSLoadStoreBonding());
 
   if (EnablePostMISchedLoadStoreClustering ||
-      EnableLoadStoreClusteringForLoadStorePairOpt) {
+      EnableLoadStoreClusteringForLoadStoreOpt) {
     DAG = createGenericSchedPostRA(C);
     DAG->addMutation(createLoadClusterDAGMutation(
         DAG->TII, DAG->TRI, /*ReorderWhileClustering=*/true));
@@ -574,7 +575,7 @@ void RISCVPassConfig::addPreEmitPass() {
 
   // LoadStoreOptimizer creates bundles for load-store bonding.
   addPass(createUnpackMachineBundles([](const MachineFunction &MF) {
-    return MF.getSubtarget<RISCVSubtarget>().useLoadStorePairs();
+    return MF.getSubtarget<RISCVSubtarget>().useMIPSLoadStoreBonding();
   }));
 }
 

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

@@ -181,7 +181,7 @@
 ; CHECK-NEXT:       Post-RA pseudo instruction expansion pass
 ; CHECK-NEXT:       RISC-V post-regalloc pseudo instruction expansion pass
 ; CHECK-NEXT:       Insert KCFI indirect call checks
-; CHECK-NEXT:       RISCV Load / Store Optimizer
+; CHECK-NEXT:       RISC-V Load / Store Optimizer
 ; CHECK-NEXT:       MachineDominator Tree Construction
 ; CHECK-NEXT:       Machine Natural Loop Construction
 ; CHECK-NEXT:       PostRA Machine Instruction Scheduler

llvm/test/CodeGen/RISCV/load-store-pair.ll

@@ -1,4 +1,5 @@
 ; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; testd and testf look for bonding only.
 ; RUN: llc -mtriple=riscv32 -verify-machineinstrs < %s \
 ; RUN:   | FileCheck %s -check-prefix=RV32I
 ; RUN: llc -mtriple=riscv32 -target-abi ilp32d -mattr=+d -verify-machineinstrs < %s \