[PATCH] D135009: [RISCV] Refactor and improve eliminateFrameIndex.

[Philip Reames via Phabricator via llvm-commits]

reames added inline comments.


================
Comment at: llvm/lib/Target/RISCV/RISCVRegisterInfo.cpp:187
+  Register DestReg;
+  if (MI.getOpcode() == RISCV::ADDI)
+    DestReg = MI.getOperand(0).getReg();
----------------
I don't follow this logic.  Why is knowing it's an add enough to know we can reuse/rewrite it?   Actually, I see this part is not new in the code, it's just moved up from above.  I'd still encourage you to improve the comment.  :)


================
Comment at: llvm/lib/Target/RISCV/RISCVRegisterInfo.cpp:241
+  // Add in the scalable offset which has already been computed in DestReg.
+  if (Offset.getScalable()) {
+    assert(DestReg && "DestReg should be valid");
----------------
Can you land a pre-change which just does the change in code structure here?  This diff is very hard to read.  No separate review needed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135009