[PATCH] D144336: [GlobalISel] Fix DIVREM combine from inserting a divrem before its operands' defs.

[Amara Emerson via Phabricator via llvm-commits]

aemerson added inline comments.


================
Comment at: llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp:1238
                      {DestDivReg, DestRemReg},
                      {MI.getOperand(1).getReg(), MI.getOperand(2).getReg()});
   MI.eraseFromParent();
----------------
efriedma wrote:
> The reason this breaks is that you're using `MI.getOperand(1).getReg()`, and that doesn't dominate the insertion position, right?  But `OtherMI->getOperand(1).getReg()` is equivalent, and if we're inserting at the position of OtherMI, it should dominate the insert position, I think?
Might have misunderstood your comment, but we're not necessarily inserting at OtherMI. We have to insert at whichever of the pair is first in block order to not break use-defs. If any def of second's sources are dominated by the first, then there's no safe place to choose (without more careful analysis).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144336