[PATCH] D82988: [RISCV] Avoid Splitting MBB in RISCVExpandPseudo

[Sam Elliott via Phabricator via llvm-commits]

lenary marked an inline comment as done.
lenary added a comment.

The RISCVTargetMachine changes are not distinct. You can only use virtual registers (ScratchReg) if you are before register allocation, as I understand it.



================
Comment at: llvm/lib/Target/RISCV/RISCVExpandPseudoInsts.cpp:74-76
     MachineBasicBlock::iterator NMBBI = std::next(MBBI);
-    Modified |= expandMI(MBB, MBBI, NMBBI);
+    Modified |= expandMI(MBB, MBBI);
     MBBI = NMBBI;
----------------
luismarques wrote:
> lenary wrote:
> > Oh this loop can be simplifed - though I'm not sure I should be incrementing `MBBI` if we've inserted new instructions using `BuildMI`, which I think also increments the iterator automatically. Guidance here would be helpful.
> Maybe I misunderstood your issue, but I think the increment is correct:
> 1) It is what AArch64 does.
> 2) If you have two consecutive pseudo-instructions this code expands them both, so you aren't skipping over the second by performing the explicit increment.
> 3) You don't want to iterate over the expanded instructions, to recursively expand them, since we don't emit other pseudo-instructions in the expansions.
> BTW, what simplification were you considering for this loop?
Turning it back into a for loop like:
```
for (MachineBasicBlock::iterator MBBI = MBB.begin(), E = MBB.end(); MBBI != E; MBBI++) {
  Modified |= expandMI(MBB, MBBI)
}
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82988