[RISCV] Fix isStoreToStackSlot

[rogfer01]

Summary

Because of the layout of stores (that don't have a destination operand) this check is exactly the same as the one in RISCVInstrInfo::isLoadFromStackSlot.

I discovered this while doing some experiments that needed identifying loads/stores from/to the stack. Stores always return false.

[rogfer01]

I have been unable to come up with a test that shows any change (I may check LNT to see if something changes), so ideas are welcome here.

However I'd expect this function return true (at least for now in RISC-V) for every instruction created in RISCVInstrInfo::storeRegToStackSlot. Conceptually (not part of the current patch) like this.

diff --git a/llvm/lib/Target/RISCV/RISCVInstrInfo.cpp b/llvm/lib/Target/RISCV/RISCVInstrInfo.cpp
index dc212d9cde2..8a28a21a29e 100644
--- a/llvm/lib/Target/RISCV/RISCVInstrInfo.cpp
+++ b/llvm/lib/Target/RISCV/RISCVInstrInfo.cpp
@@ -131,10 +131,14 @@ void RISCVInstrInfo::storeRegToStackSlot(MachineBasicBlock &MBB,
   else
     llvm_unreachable("Can't store this register to stack slot");
 
-  BuildMI(MBB, I, DL, get(Opcode))
+  MachineInstr *MI = BuildMI(MBB, I, DL, get(Opcode))
       .addReg(SrcReg, getKillRegState(IsKill))
       .addFrameIndex(FI)
       .addImm(0);
+
+  int Dummy;
+  (void)Dummy;
+  assert(this->isStoreToStackSlot(*MI, Dummy) && "This is exactly a store to a stack slot");
 }

[luismarques]

LGTM. Good catch Roger!
(I have verified that the code change makes sense based both on tablegen definitions and the sanity test that Roger indicated on the comment).

[asb]

This looks good to me, good catch. I do see a codegen change on regstack-1.c from the GCC Torture Suite, so it might be worth having a quick look to see if it's easy to make a test case based on that.

[rogfer01]

ChangeLog:

• Update test from D83750

[rogfer01]

ChangeLog:

• Rebase

llvm/lib/Target/RISCV/RISCVInstrInfo.cpp

unsigned RISCVInstrInfo::isStoreToStackSlot(const MachineInstr &MI,
   case RISCV::SH:
   case RISCV::SW:
   case RISCV::FSW:
   case RISCV::SD:
   case RISCV::FSD:
     break;
   }
 
-  if (MI.getOperand(0).isFI() && MI.getOperand(1).isImm() &&
-      MI.getOperand(1).getImm() == 0) {
-    FrameIndex = MI.getOperand(0).getIndex();
-    return MI.getOperand(2).getReg();
+  if (MI.getOperand(1).isFI() && MI.getOperand(2).isImm() &&
+      MI.getOperand(2).getImm() == 0) {
+    FrameIndex = MI.getOperand(1).getIndex();
+    return MI.getOperand(0).getReg();
   }
 
   return 0;
 }
 
 void RISCVInstrInfo::copyPhysReg(MachineBasicBlock &MBB,
                                  MachineBasicBlock::iterator MBBI,
                                  const DebugLoc &DL, MCRegister DstReg,

llvm/test/CodeGen/RISCV/stack-store-check.ll

 ; CHECK-NEXT:    sw a2, 52(sp)
 ; CHECK-NEXT:    lw a3, %lo(Y1+8)(a0)
 ; CHECK-NEXT:    sw a3, 4(sp)
 ; CHECK-NEXT:    lw a0, %lo(Y1+12)(a0)
 ; CHECK-NEXT:    sw a0, 0(sp)
 ; CHECK-NEXT:    sw a0, 308(sp)
 ; CHECK-NEXT:    sw a3, 304(sp)
 ; CHECK-NEXT:    sw a2, 300(sp)
-; CHECK-NEXT:    lw a0, 52(sp)
 ; CHECK-NEXT:    sw a1, 296(sp)
 ; CHECK-NEXT:    sw s11, 324(sp)
 ; CHECK-NEXT:    sw s9, 320(sp)
 ; CHECK-NEXT:    sw s4, 316(sp)
 ; CHECK-NEXT:    addi a0, sp, 328
 ; CHECK-NEXT:    addi a1, sp, 312
 ; CHECK-NEXT:    addi a2, sp, 296
 ; CHECK-NEXT:    sw s3, 312(sp)