[PATCH] D75098: Add COPY_BR, a terminator form of the COPY instr

Nick Desaulniers via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Feb 25 13:39:28 PST 2020


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

Also, if INLINEASM_BR is a terminator, but we want TCOPY to be a terminator, should this patch make INLINEASM_BR NOT a terminator? Or would that be a follow up patch after creating TCOPY?

Also, none of the tests have TCOPY (or w/e). I feel like any patch adding a new instructions at w/e level of IR should have some test that the instruction exists.

Also, I don't quite follow what the changes to the tests are testing for with this change.



================
Comment at: llvm/lib/CodeGen/ExpandPostRAPseudos.cpp:191
+  for (auto &MBB : MF) {
+    for (MachineBasicBlock::iterator mi = MBB.begin(), me = MBB.end();
          mi != me;) {
----------------
void wrote:
> nickdesaulniers wrote:
> > what? if we could use a range based for loop for the `MBB`'s, surely we can for `MI`.
> The `++mi` below made me worried, and the fact that MI could be erased.
Ah sorry, I missed that.  I'm not sure what's meant by `erased`, but I'd be curious if a `continue` could not be used instead of manual iterator advancement.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75098





More information about the llvm-commits mailing list