[PATCH] D124824: [RISCV] Make use of SHXADD instructions in RVV spill/reload code.

[Craig Topper via Phabricator via llvm-commits]

craig.topper added inline comments.


================
Comment at: llvm/lib/Target/RISCV/RISCVInstrInfo.cpp:1760
                                                MachineInstr::MIFlag Flag) const {
   assert(Amount > 0 && "There is no need to get VLEN scaled value.");
   assert(Amount % 8 == 0 &&
----------------
reames wrote:
> craig.topper wrote:
> > I'm considering merging this with it's one caller. There might be some more optimization opportunities if we include the ADD/SUB to SP.
> > 
> > For example, vlenb * 7 + sp could become
> > 
> > ```
> > csrrd a0, vlenb
> > sub sp, sp, a0
> > slli a0, a0, 3
> > add sp, sp, a0
> > ```
> > 
> > instead of
> > 
> > ```
> > csrrd a0, vlenb
> > slli a1, a0, 3
> > sub a0, a1, a0
> > add sp, sp, a0
> > ```
> > 
> > That would save a temporary register.
> Separate commit please.  :)
> 
> Any chance this could be done in a post-process combine like thing?  Duplicating what appears to be generic multiple improvements seems like poor code structure.
Yes separate commit.

I'm not sure if we could do it in a generic combine. This code runs after register allocation and needs to scavenge a free register. An emergency spill will happen if it can't find one. Avoiding emergency spills would be one of the benefits of saving the use of a temporary register. If did a generic combine later we'd also have to remove the emergency spill.


================
Comment at: llvm/lib/Target/RISCV/RISCVInstrInfo.cpp:1765
   MachineRegisterInfo &MRI = MF.getRegInfo();
-  const RISCVInstrInfo *TII = MF.getSubtarget<RISCVSubtarget>().getInstrInfo();
   int64_t NumOfVReg = Amount / 8;
----------------
reames wrote:
> Can you commit this separately as a cleanup please?
Will do


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124824