[RISCV] remove redundant instruction when eliminate frame index

[StephenFan]

Summary

The reason for generating mv a0, a0 instruction is when the stack object offset is large then int<12>. To deal this situation, in the elimintateFrameIndex function, it will
create a virtual register, which needs the register scavenger to scavenge it. If the machine instruction that contains the stack object and the opcode is ADDI(the addi
was generated by frameindexNode), and then this instruction's destination register was the same as the register that was generated by the register scavenger, then the
mv a0, a0 was generated. So to eliminnate this instruction, in the eliminateFrameIndex function, if the instrution opcode is ADDI, then the virtual register can't be created

[jrtc27]

This is a bit ugly. Surely a simple peephole optimisation should be able to fix this and any other cases more generally? But also please give this a better title and commit message.

[jrtc27]

It's also wrong if the ADDI has FrameReg as its destination. A neater approach might be to always use the destination register (if there is one, using general instruction query information) as the destination for the ADD, keeping the scratch register for the movImm call. Then you'd only need special logic to work out whether you need to keep the (modified) existing instruction.

[StephenFan]

In D92479#2428454, @jrtc27 wrote:

It's also wrong if the ADDI has FrameReg as its destination. A neater approach might be to always use the destination register (if there is one, using general instruction query information) as the destination for the ADD, keeping the scratch register for the movImm call. Then you'd only need special logic to work out whether you need to keep the (modified) existing instruction.

I agree that it is wrong if the ADDI has FrameReg as its destination. And I adapted your suggestion.

[kito-cheng]

llvm/lib/Target/RISCV/RISCVRegisterInfo.cpp

216

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?

[StephenFan]

llvm/lib/Target/RISCV/RISCVRegisterInfo.cpp

216

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.

[StephenFan]

ping...

[jrtc27]

llvm/lib/Target/RISCV/RISCVRegisterInfo.cpp

216

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.

[StephenFan]

llvm/lib/Target/RISCV/RISCVRegisterInfo.cpp

216

Do you talk about what RISCVDAGToDAGISel::doPeepholeLoadStoreADDI function do ?

[StephenFan]

Ping.

[asb]

This needs a rebase, but LGTM to land after that. Thanks!

llvm/lib/Target/RISCV/RISCVRegisterInfo.cpp

void RISCVRegisterInfo::eliminateFrameIndex(MachineBasicBlock::iterator II,
   MachineBasicBlock &MBB = *MI.getParent();
   bool FrameRegIsKill = false;
 
   if (!isInt<12>(Offset.getFixed())) {
     // The offset won't fit in an immediate, so use a scratch register instead
     // Modify Offset and FrameReg appropriately
     Register ScratchReg = MRI.createVirtualRegister(&RISCV::GPRRegClass);
     TII->movImm(MBB, II, DL, ScratchReg, Offset.getFixed());
+    if (MI.getOpcode() == RISCV::ADDI) {
+      BuildMI(MBB, II, DL, TII->get(RISCV::ADD), MI.getOperand(0).getReg())
+        .addReg(FrameReg)
+        .addReg(ScratchReg, RegState::Kill);
+      MI.eraseFromParent();
+      return;
+    }
     BuildMI(MBB, II, DL, TII->get(RISCV::ADD), ScratchReg)
         .addReg(FrameReg)
         .addReg(ScratchReg, RegState::Kill);
     Offset = StackOffset::get(0, Offset.getScalable());
     FrameReg = ScratchReg;
     FrameRegIsKill = true;
   }
 

llvm/test/CodeGen/RISCV/large-stack.ll

 ; RV32I-WITHFP-NEXT:    lui a1, 97
 ; RV32I-WITHFP-NEXT:    addi a1, a1, 688
 ; RV32I-WITHFP-NEXT:    sub sp, sp, a1
 ; RV32I-WITHFP-NEXT:    lui a1, 78
 ; RV32I-WITHFP-NEXT:    addi a1, a1, 512
 ; RV32I-WITHFP-NEXT:    lui a2, 1048478
 ; RV32I-WITHFP-NEXT:    addi a2, a2, 1388
 ; RV32I-WITHFP-NEXT:    add a2, s0, a2
-; RV32I-WITHFP-NEXT:    mv a2, a2
 ; RV32I-WITHFP-NEXT:    add a1, a2, a1
 ; RV32I-WITHFP-NEXT:    #APP
 ; RV32I-WITHFP-NEXT:    nop
 ; RV32I-WITHFP-EMPTY:
 ; RV32I-WITHFP-NEXT:    #NO_APP
 ; RV32I-WITHFP-NEXT:    sw a0, 0(a1)
 ; RV32I-WITHFP-NEXT:    #APP
 ; RV32I-WITHFP-NEXT:    nop

llvm/test/CodeGen/RISCV/stack-realignment.ll

 ; RV32I-NEXT:    .cfi_def_cfa s0, 0
 ; RV32I-NEXT:    lui a0, 1
 ; RV32I-NEXT:    addi a0, a0, -2032
 ; RV32I-NEXT:    sub sp, sp, a0
 ; RV32I-NEXT:    andi sp, sp, -2048
 ; RV32I-NEXT:    lui a0, 1
 ; RV32I-NEXT:    addi a0, a0, -2048
 ; RV32I-NEXT:    add a0, sp, a0
-; RV32I-NEXT:    mv a0, a0
 ; RV32I-NEXT:    call callee@plt
 ; RV32I-NEXT:    lui a0, 1
 ; RV32I-NEXT:    sub sp, s0, a0
 ; RV32I-NEXT:    lui a0, 1
 ; RV32I-NEXT:    addi a0, a0, -2032
 ; RV32I-NEXT:    add sp, sp, a0
 ; RV32I-NEXT:    lw s0, 2024(sp) # 4-byte Folded Reload
 ; RV32I-NEXT:    lw ra, 2028(sp) # 4-byte Folded Reload

 ; RV64I-NEXT:    .cfi_def_cfa s0, 0
 ; RV64I-NEXT:    lui a0, 1
 ; RV64I-NEXT:    addiw a0, a0, -2032
 ; RV64I-NEXT:    sub sp, sp, a0
 ; RV64I-NEXT:    andi sp, sp, -2048
 ; RV64I-NEXT:    lui a0, 1
 ; RV64I-NEXT:    addiw a0, a0, -2048
 ; RV64I-NEXT:    add a0, sp, a0
-; RV64I-NEXT:    mv a0, a0
 ; RV64I-NEXT:    call callee@plt
 ; RV64I-NEXT:    lui a0, 1
 ; RV64I-NEXT:    sub sp, s0, a0
 ; RV64I-NEXT:    lui a0, 1
 ; RV64I-NEXT:    addiw a0, a0, -2032
 ; RV64I-NEXT:    add sp, sp, a0
 ; RV64I-NEXT:    ld s0, 2016(sp) # 8-byte Folded Reload
 ; RV64I-NEXT:    ld ra, 2024(sp) # 8-byte Folded Reload

 ; RV32I-NEXT:    .cfi_def_cfa s0, 0
 ; RV32I-NEXT:    lui a0, 2
 ; RV32I-NEXT:    addi a0, a0, -2032
 ; RV32I-NEXT:    sub sp, sp, a0
 ; RV32I-NEXT:    srli a0, sp, 12
 ; RV32I-NEXT:    slli sp, a0, 12
 ; RV32I-NEXT:    lui a0, 1
 ; RV32I-NEXT:    add a0, sp, a0
-; RV32I-NEXT:    mv a0, a0
 ; RV32I-NEXT:    call callee@plt
 ; RV32I-NEXT:    lui a0, 2
 ; RV32I-NEXT:    sub sp, s0, a0
 ; RV32I-NEXT:    lui a0, 2
 ; RV32I-NEXT:    addi a0, a0, -2032
 ; RV32I-NEXT:    add sp, sp, a0
 ; RV32I-NEXT:    lw s0, 2024(sp) # 4-byte Folded Reload
 ; RV32I-NEXT:    lw ra, 2028(sp) # 4-byte Folded Reload

 ; RV64I-NEXT:    .cfi_def_cfa s0, 0
 ; RV64I-NEXT:    lui a0, 2
 ; RV64I-NEXT:    addiw a0, a0, -2032
 ; RV64I-NEXT:    sub sp, sp, a0
 ; RV64I-NEXT:    srli a0, sp, 12
 ; RV64I-NEXT:    slli sp, a0, 12
 ; RV64I-NEXT:    lui a0, 1
 ; RV64I-NEXT:    add a0, sp, a0
-; RV64I-NEXT:    mv a0, a0
 ; RV64I-NEXT:    call callee@plt
 ; RV64I-NEXT:    lui a0, 2
 ; RV64I-NEXT:    sub sp, s0, a0
 ; RV64I-NEXT:    lui a0, 2
 ; RV64I-NEXT:    addiw a0, a0, -2032
 ; RV64I-NEXT:    add sp, sp, a0
 ; RV64I-NEXT:    ld s0, 2016(sp) # 8-byte Folded Reload
 ; RV64I-NEXT:    ld ra, 2024(sp) # 8-byte Folded Reload

llvm/test/CodeGen/RISCV/vararg.ll

 ; ILP32-ILP32F-FPELIM-NEXT:    sw a2, 0(a3)
 ; ILP32-ILP32F-FPELIM-NEXT:    lui a2, 24414
 ; ILP32-ILP32F-FPELIM-NEXT:    addi a2, a2, 276
 ; ILP32-ILP32F-FPELIM-NEXT:    add a2, sp, a2
 ; ILP32-ILP32F-FPELIM-NEXT:    sw a1, 0(a2)
 ; ILP32-ILP32F-FPELIM-NEXT:    lui a1, 24414
 ; ILP32-ILP32F-FPELIM-NEXT:    addi a1, a1, 280
 ; ILP32-ILP32F-FPELIM-NEXT:    add a1, sp, a1
-; ILP32-ILP32F-FPELIM-NEXT:    mv a1, a1
 ; ILP32-ILP32F-FPELIM-NEXT:    sw a1, 12(sp)
 ; ILP32-ILP32F-FPELIM-NEXT:    lui a1, 24414
 ; ILP32-ILP32F-FPELIM-NEXT:    addi a1, a1, 304
 ; ILP32-ILP32F-FPELIM-NEXT:    add sp, sp, a1
 ; ILP32-ILP32F-FPELIM-NEXT:    ret
 ;
 ; ILP32-ILP32F-WITHFP-LABEL: va_large_stack:
 ; ILP32-ILP32F-WITHFP:       # %bb.0:

 ; RV32D-ILP32-ILP32F-ILP32D-FPELIM-NEXT:    sw a2, 0(a3)
 ; RV32D-ILP32-ILP32F-ILP32D-FPELIM-NEXT:    lui a2, 24414
 ; RV32D-ILP32-ILP32F-ILP32D-FPELIM-NEXT:    addi a2, a2, 276
 ; RV32D-ILP32-ILP32F-ILP32D-FPELIM-NEXT:    add a2, sp, a2
 ; RV32D-ILP32-ILP32F-ILP32D-FPELIM-NEXT:    sw a1, 0(a2)
 ; RV32D-ILP32-ILP32F-ILP32D-FPELIM-NEXT:    lui a1, 24414
 ; RV32D-ILP32-ILP32F-ILP32D-FPELIM-NEXT:    addi a1, a1, 280
 ; RV32D-ILP32-ILP32F-ILP32D-FPELIM-NEXT:    add a1, sp, a1
-; RV32D-ILP32-ILP32F-ILP32D-FPELIM-NEXT:    mv a1, a1
 ; RV32D-ILP32-ILP32F-ILP32D-FPELIM-NEXT:    sw a1, 12(sp)
 ; RV32D-ILP32-ILP32F-ILP32D-FPELIM-NEXT:    lui a1, 24414
 ; RV32D-ILP32-ILP32F-ILP32D-FPELIM-NEXT:    addi a1, a1, 304
 ; RV32D-ILP32-ILP32F-ILP32D-FPELIM-NEXT:    add sp, sp, a1
 ; RV32D-ILP32-ILP32F-ILP32D-FPELIM-NEXT:    ret
 ;
 ; LP64-LP64F-LP64D-FPELIM-LABEL: va_large_stack:
 ; LP64-LP64F-LP64D-FPELIM:       # %bb.0:

 ; LP64-LP64F-LP64D-FPELIM-NEXT:    sd a3, 0(a0)
 ; LP64-LP64F-LP64D-FPELIM-NEXT:    lui a0, 24414
 ; LP64-LP64F-LP64D-FPELIM-NEXT:    addiw a0, a0, 288
 ; LP64-LP64F-LP64D-FPELIM-NEXT:    add a0, sp, a0
 ; LP64-LP64F-LP64D-FPELIM-NEXT:    sd a2, 0(a0)
 ; LP64-LP64F-LP64D-FPELIM-NEXT:    lui a0, 24414
 ; LP64-LP64F-LP64D-FPELIM-NEXT:    addiw a0, a0, 284
 ; LP64-LP64F-LP64D-FPELIM-NEXT:    add a0, sp, a0
-; LP64-LP64F-LP64D-FPELIM-NEXT:    mv a0, a0
 ; LP64-LP64F-LP64D-FPELIM-NEXT:    sd a0, 8(sp)
 ; LP64-LP64F-LP64D-FPELIM-NEXT:    lui a0, 24414
 ; LP64-LP64F-LP64D-FPELIM-NEXT:    addiw a0, a0, 280
 ; LP64-LP64F-LP64D-FPELIM-NEXT:    add a0, sp, a0
 ; LP64-LP64F-LP64D-FPELIM-NEXT:    lw a0, 0(a0)
 ; LP64-LP64F-LP64D-FPELIM-NEXT:    lui a1, 24414
 ; LP64-LP64F-LP64D-FPELIM-NEXT:    addiw a1, a1, 336
 ; LP64-LP64F-LP64D-FPELIM-NEXT:    add sp, sp, a1