[PATCH] D81805: [RISCV] Fix isStoreToStackSlot

[Roger Ferrer Ibanez via Phabricator via llvm-commits]

rogfer01 created this revision.
rogfer01 added reviewers: asb, lenary, luismarques.
Herald added subscribers: llvm-commits, evandro, apazos, sameer.abuasal, pzheng, s.egerton, Jim, benna, psnobl, jocewei, PkmX, rkruppe, the_o, brucehoult, MartinMosbeck, edward-jones, zzheng, MaskRay, shiva0217, kito-cheng, niosHD, sabuasal, simoncook, johnrusso, rbar, hiraditya.
Herald added a project: LLVM.
rogfer01 added a comment.
rogfer01 set the repository for this revision to rG LLVM Github Monorepo.
Herald added a subscriber: jrtc27.

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");
   }


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.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D81805

Files:
  llvm/lib/Target/RISCV/RISCVInstrInfo.cpp


Index: llvm/lib/Target/RISCV/RISCVInstrInfo.cpp
===================================================================
--- llvm/lib/Target/RISCV/RISCVInstrInfo.cpp
+++ llvm/lib/Target/RISCV/RISCVInstrInfo.cpp
@@ -76,10 +76,10 @@
     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;


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D81805.270622.patch
Type: text/x-patch
Size: 643 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200614/d662e849/attachment.bin>