[PATCH] D138926: [RISCV] Reuse and generalize adjustReg from another spot in frame lowering [nfc]

Philip Reames via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Nov 30 09:37:40 PST 2022


reames added inline comments.


================
Comment at: llvm/lib/Target/RISCV/RISCVFrameLowering.cpp:350
+  const RISCVRegisterInfo &RI = *STI.getRegisterInfo();
+  RI.adjustReg(MBB, MBBI, DL, SPReg, SPReg, StackOffset::getScalable(Amount),
+               Flag);
----------------
frasercrmck wrote:
> We have `RISCVFrameLowering::adjustReg` (used elsewhere in this file) which wraps the call to `RI.adjustReg`. I think we should continue to use use `adjustReg` (for now) even though they're equivalent.
> 
> Personally I'm not a huge fan of having identically-named methods across multiple files/classes. So if you wanted to remove `RISCVFrameLowering::adjustReg` and make everyone call the `RISCVRegisterInfo` one directly, I'd support that.
This is certainly the end goal, and after this patch and D132839
 which just landed, I think I'm close to being able to do that.  Might be one or two more cleanup changes first, but that's definitely where we're heading.  


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138926



More information about the llvm-commits mailing list