[RISCV] Improve load/store pairing for Xqcilsm instructions in RISCVLoadStoreOptimizer

[svs-quic]

If we did not pair two viable Xqcilsm load/store instructions in RISCVLoadStoreOptimizer we iterated over the second instruction when looking for other pairs to merge. This leads to us missing out on opportunities where the second instruction could have been the start of another potential pair that could have been merged.

[llvmbot]

@llvm/pr-subscribers-backend-risc-v

Author: Sudharsan Veeravalli (svs-quic)

Changes

If we did not pair two viable Xqcilsm load/store instructions in RISCVLoadStoreOptimizer we iterated over the second instruction when looking for other pairs to merge. This leads to us missing out on opportunities where the second instruction could have been the start of another potential pair that could have been merged.

---

Full diff: https://github.com/llvm/llvm-project/pull/174612.diff

2 Files Affected:

• (modified) llvm/lib/Target/RISCV/RISCVLoadStoreOptimizer.cpp (+14-4)
• (modified) llvm/test/CodeGen/RISCV/xqcilsm-lwmi-swmi.mir (+57)

diff --git a/llvm/lib/Target/RISCV/RISCVLoadStoreOptimizer.cpp b/llvm/lib/Target/RISCV/RISCVLoadStoreOptimizer.cpp
index a5e12c0a51112..0c98dbbe4cadf 100644
--- a/llvm/lib/Target/RISCV/RISCVLoadStoreOptimizer.cpp
+++ b/llvm/lib/Target/RISCV/RISCVLoadStoreOptimizer.cpp
@@ -482,10 +482,13 @@ RISCVLoadStoreOpt::mergePairedInsns(MachineBasicBlock::iterator I,
                                     bool MergeForward) {
   MachineBasicBlock::iterator E = I->getParent()->end();
   MachineBasicBlock::iterator NextI = next_nodbg(I, E);
-  // If NextI is the second of the two instructions to be merged, we need
-  // to skip one further. Either way we merge will invalidate the iterator,
-  // and we don't need to scan the new instruction, as it's a pairwise
-  // instruction, which we're not considering for further action anyway.
+  // If NextI is the second of the two instructions to be merged, skip one
+  // further for now. For the MIPS load/store, the merge will invalidate the
+  // iterator, and we don't need to scan the new instruction, as it's a pairwise
+  // instruction, which we're not considering for further action anyway. For the
+  // Xqcilsm load/store, we may not want to do this as the second instruction
+  // could possibly be the first in another pair if we do not merge here. This
+  // is handled is the else block after the call to tryConvertToLdStPair below.
   if (NextI == Paired)
     NextI = next_nodbg(NextI, E);
 
@@ -535,9 +538,16 @@ RISCVLoadStoreOpt::mergePairedInsns(MachineBasicBlock::iterator I,
     First = InsertionPoint;
   }
 
+  MachineFunction *MF = I->getMF();
+  const RISCVSubtarget &STI = MF->getSubtarget<RISCVSubtarget>();
+
   if (tryConvertToLdStPair(First, Second)) {
     LLVM_DEBUG(dbgs() << "Pairing load/store:\n    ");
     LLVM_DEBUG(prev_nodbg(NextI, MBB.begin())->print(dbgs()));
+  } else if (!STI.is64Bit() && STI.hasVendorXqcilsm()) {
+    // We were unable to form the pair, so use the next non-debug instruction
+    // after the first instruction we had wanted to merge.
+    NextI = next_nodbg(I, E);
   }
 
   return NextI;
diff --git a/llvm/test/CodeGen/RISCV/xqcilsm-lwmi-swmi.mir b/llvm/test/CodeGen/RISCV/xqcilsm-lwmi-swmi.mir
index 1bb6599bb0e0f..971a62dbc0323 100644
--- a/llvm/test/CodeGen/RISCV/xqcilsm-lwmi-swmi.mir
+++ b/llvm/test/CodeGen/RISCV/xqcilsm-lwmi-swmi.mir
@@ -19,6 +19,9 @@
   define void @no_pair_if_rd_is_x0_swmi() nounwind { ret void }
   define void @pair_if_rd_is_x0_setwmi() nounwind { ret void }
   define void @no_pair_if_lw_rd_equals_base() nounwind { ret void }
+  define void @second_pair_if_no_first_pair_lwmi() nounwind { ret void }
+  define void @second_pair_if_no_first_pair_swmi() nounwind { ret void }
+  define void @second_pair_if_no_first_pair_setwmi() nounwind { ret void }
   define void @pair_if_not_adjacent() nounwind { ret void }
   define void @pair_if_not_adjacent_use() nounwind { ret void }
   define void @no_pair_if_not_adjacent_use() nounwind { ret void }
@@ -292,6 +295,60 @@ body: |
     $x11 = LW $x10, 24 :: (load (s32), align 4)
     PseudoRET
 
+...
+---
+name: second_pair_if_no_first_pair_lwmi
+tracksRegLiveness: false
+body: |
+  bb.0:
+    liveins: $x10
+    ; CHECK-LABEL: name: second_pair_if_no_first_pair_lwmi
+    ; CHECK: liveins: $x10
+    ; CHECK-NEXT: {{  $}}
+    ; CHECK-NEXT: $x0 = LW $x10, 0 :: (load (s32))
+    ; CHECK-NEXT: $x1 = QC_LWMI $x10, 2, 4, implicit-def $x2 :: (load (s32))
+    ; CHECK-NEXT: PseudoRET
+    $x0 = LW $x10, 0 :: (load (s32), align 4)
+    $x1 = LW $x10, 4 :: (load (s32), align 4)
+    $x2 = LW $x10, 8 :: (load (s32), align 4)
+    PseudoRET
+
+...
+---
+name: second_pair_if_no_first_pair_swmi
+tracksRegLiveness: false
+body: |
+  bb.0:
+    liveins: $x10
+    ; CHECK-LABEL: name: second_pair_if_no_first_pair_swmi
+    ; CHECK: liveins: $x10
+    ; CHECK-NEXT: {{  $}}
+    ; CHECK-NEXT: SW $x0, $x10, 0 :: (store (s32))
+    ; CHECK-NEXT: QC_SWMI $x1, $x10, 2, 4, implicit $x2 :: (store (s32))
+    ; CHECK-NEXT: PseudoRET
+    SW $x0, $x10, 0 :: (store (s32), align 4)
+    SW $x1, $x10, 4 :: (store (s32), align 4)
+    SW $x2, $x10, 8 :: (store (s32), align 4)
+    PseudoRET
+
+...
+---
+name: second_pair_if_no_first_pair_setwmi
+tracksRegLiveness: false
+body: |
+  bb.0:
+    liveins: $x10
+    ; CHECK-LABEL: name: second_pair_if_no_first_pair_setwmi
+    ; CHECK: liveins: $x10
+    ; CHECK-NEXT: {{  $}}
+    ; CHECK-NEXT: SW $x0, $x10, 0 :: (store (s32))
+    ; CHECK-NEXT: QC_SETWMI $x2, $x10, 2, 4 :: (store (s32))
+    ; CHECK-NEXT: PseudoRET
+    SW $x0, $x10, 0 :: (store (s32), align 4)
+    SW $x2, $x10, 4 :: (store (s32), align 4)
+    SW $x2, $x10, 8 :: (store (s32), align 4)
+    PseudoRET
+
 ...
 ---
 # FIXME: Kill flags are not propagated correctly for the base register

[github-actions]

🪟 Windows x64 Test Results

• 129177 tests passed
• 2843 tests skipped
• 1 test failed

Failed Tests

(click on a test name to see its output)

LLVM

LLVM.CodeGen/RISCV/riscv-macho.ll

Exit Code: 3221225477

Command Output (stdout):
--
# RUN: at line 1
c:\_work\llvm-project\llvm-project\build\bin\llc.exe -mtriple=riscv32-apple-macho C:\_work\llvm-project\llvm-project\llvm\test\CodeGen\RISCV\riscv-macho.ll -o - | c:\_work\llvm-project\llvm-project\build\bin\filecheck.exe C:\_work\llvm-project\llvm-project\llvm\test\CodeGen\RISCV\riscv-macho.ll
# executed command: 'c:\_work\llvm-project\llvm-project\build\bin\llc.exe' -mtriple=riscv32-apple-macho 'C:\_work\llvm-project\llvm-project\llvm\test\CodeGen\RISCV\riscv-macho.ll' -o -
# note: command had no output on stdout or stderr
# executed command: 'c:\_work\llvm-project\llvm-project\build\bin\filecheck.exe' 'C:\_work\llvm-project\llvm-project\llvm\test\CodeGen\RISCV\riscv-macho.ll'
# note: command had no output on stdout or stderr
# RUN: at line 2
c:\_work\llvm-project\llvm-project\build\bin\llc.exe -mtriple=riscv32-apple-macho -filetype=obj C:\_work\llvm-project\llvm-project\llvm\test\CodeGen\RISCV\riscv-macho.ll -o C:\_work\llvm-project\llvm-project\build\test\CodeGen\RISCV\Output\riscv-macho.ll.tmp.o
# executed command: 'c:\_work\llvm-project\llvm-project\build\bin\llc.exe' -mtriple=riscv32-apple-macho -filetype=obj 'C:\_work\llvm-project\llvm-project\llvm\test\CodeGen\RISCV\riscv-macho.ll' -o 'C:\_work\llvm-project\llvm-project\build\test\CodeGen\RISCV\Output\riscv-macho.ll.tmp.o'
# .---command stderr------------
# | PLEASE submit a bug report to https://github.com/llvm/llvm-project/issues/ and include the crash backtrace and instructions to reproduce the bug.
# | Stack dump:
# | 0.	Program arguments: c:\\_work\\llvm-project\\llvm-project\\build\\bin\\llc.exe -mtriple=riscv32-apple-macho -filetype=obj C:\\_work\\llvm-project\\llvm-project\\llvm\\test\\CodeGen\\RISCV\\riscv-macho.ll -o C:\\_work\\llvm-project\\llvm-project\\build\\test\\CodeGen\\RISCV\\Output\\riscv-macho.ll.tmp.o
# | Exception Code: 0xC0000005
# |  #0 0x00007fff8b312004 (C:\Windows\SYSTEM32\ntdll.dll+0x12004)
# |  #1 0x00007fff8b311dda (C:\Windows\SYSTEM32\ntdll.dll+0x11dda)
# |  #2 0x00007fff77ffff29 (C:\Windows\System32\ucrtbase.dll+0xff29)
# |  #3 0x00007ff60d150e79 (c:\_work\llvm-project\llvm-project\build\bin\llc.exe+0x6f0e79)
# |  #4 0x00007ff60f8b1a43 (c:\_work\llvm-project\llvm-project\build\bin\llc.exe+0x2e51a43)
# |  #5 0x00007ff60e8ea7a8 (c:\_work\llvm-project\llvm-project\build\bin\llc.exe+0x1e8a7a8)
# |  #6 0x00007ff60d2ceb75 (c:\_work\llvm-project\llvm-project\build\bin\llc.exe+0x86eb75)
# |  #7 0x00007ff60cf21a31 (c:\_work\llvm-project\llvm-project\build\bin\llc.exe+0x4c1a31)
# |  #8 0x00007ff60cf1a041 (c:\_work\llvm-project\llvm-project\build\bin\llc.exe+0x4ba041)
# |  #9 0x00007ff60ca67c0c (c:\_work\llvm-project\llvm-project\build\bin\llc.exe+0x7c0c)
# | #10 0x00007ff60ca64e86 (c:\_work\llvm-project\llvm-project\build\bin\llc.exe+0x4e86)
# | #11 0x00007ff611080ab4 (c:\_work\llvm-project\llvm-project\build\bin\llc.exe+0x4620ab4)
# | #12 0x00007fff7cc94cb0 (C:\Windows\System32\KERNEL32.DLL+0x14cb0)
# | #13 0x00007fff8b37edcb (C:\Windows\SYSTEM32\ntdll.dll+0x7edcb)
# `-----------------------------
# error: command failed with exit status: 0xc0000005

--

If these failures are unrelated to your changes (for example tests are broken or flaky at HEAD), please open an issue at https://github.com/llvm/llvm-project/issues and add the infrastructure label.

[lenary]

LGTM

[svs-quic]

Going ahead and merging this since the failure is unrelated.