[PATCH] D54846: [CodeGen][NFC] Make `TII::getMemOpBaseImmOfs` return a base operand

[Francis Visoiu Mistrih via Phabricator via llvm-commits]

thegameg marked an inline comment as done.
thegameg added inline comments.


================
Comment at: include/llvm/CodeGen/TargetInstrInfo.h:1141
   /// memory.
-  virtual bool getMemOpBaseRegImmOfs(MachineInstr &MemOp, unsigned &BaseReg,
-                                     int64_t &Offset,
-                                     const TargetRegisterInfo *TRI) const {
+  virtual bool getMemOpBaseImmOfs(MachineInstr &MI, MachineOperand *&BaseOp,
+                                  int64_t &Offset,
----------------
niravd wrote:
> sdesmalen wrote:
> > sdesmalen wrote:
> > > Is this maybe a good moment to give this a better name, e.g. getMemOperandWithOffset?
> > It seems like BaseOp should never be changed, should it be made `const MachineOperand *`?
> BaseOp should be a result value so it can't be a result. I think you meant MI which from what I've seen is never touched and should be const.
It was my first intention to do so, but a lot of APIs are not properly "constified" so I had to keep this non-const. I am planning to do that cleanup for everything in a future patch, though. Let me know if you think it's important to have it now.

The `MachineOperand` could be const (`const MachineOperand *&BaseOp`). In this case, what can't be const is the pointer (`MachineOperand *const &BaseOp`).

Regarding the MI, I decided not to do it in this patch for the same reasons above.


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

https://reviews.llvm.org/D54846