[PATCH] D92479: [RISCV] remove redundant instruction when eliminate frame index

[luxufan via Phabricator via llvm-commits]

StephenFan added inline comments.
Herald added a subscriber: vkmr.


================
Comment at: llvm/lib/Target/RISCV/RISCVRegisterInfo.cpp:174
     TII->movImm(MBB, II, DL, ScratchReg, Offset);
+    if (MI.getOpcode() == RISCV::ADDI) {
+      BuildMI(MBB, II, DL, TII->get(RISCV::ADD), MI.getOperand(0).getReg())
----------------
jrtc27 wrote:
> StephenFan wrote:
> > kito-cheng wrote:
> > > Just curios, does it possible get any other opcode than ADDI? below code are just update `Offset`, I guess that means we already assume it must be ADDI here? But I could be wrong since I didn't seriously tracing here before.
> > > 
> > > If my assumption is right then the code could be further simplified?  
> > The ADDI is generated when the frameindexSDNode be selected to ADDI
> > 
> > ```
> >   case ISD::FrameIndex: {
> >     SDValue Imm = CurDAG->getTargetConstant(0, DL, XLenVT);
> >     int FI = cast<FrameIndexSDNode>(Node)->getIndex();
> >     SDValue TFI = CurDAG->getTargetFrameIndex(FI, VT);
> >     ReplaceNode(Node, CurDAG->getMachineNode(RISCV::ADDI, DL, VT, TFI, Imm));
> >     return;
> >   }
> > ```
> > In the eliminateFrameIndex function, I think the instructions that contains the frameindex operand are
> > ADDI (show above) and load/store instruction that come from loadRegFromStackSlot and storeRegToStackSlot function. 
> If we had reg+reg addressing then we could fold the add into loads/stores, but given we don't I think this is the only case.
Do you talk about what RISCVDAGToDAGISel::doPeepholeLoadStoreADDI function do ?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D92479/new/

https://reviews.llvm.org/D92479