[PATCH] D44958: [PostRASink]Add register dependency check for implicit operands

[Francis Visoiu Mistrih via Phabricator via llvm-commits]

thegameg accepted this revision.
thegameg added a comment.
This revision is now accepted and ready to land.

In https://reviews.llvm.org/D44958#1059982, @junbuml wrote:

> I think I saw the case where LiveRegUnits is used to find a scratch reg. It seems possible to use it here as well. Let me take a little closer look at it. I think it should be a separate NFC patch.


Even without LiveRegUnits, I think you could replace ModifiedRegs and UsedRegs to be ModifiedRegUnits and UsedRegUnits, and use MCRegUnitIterators to check if registers alias.

I'm ok to commit this and follow-up in separate patches for the rest. Thanks!



================
Comment at: lib/CodeGen/MachineSink.cpp:1126
 
-    unsigned DefReg = MI->getOperand(0).getReg();
-    unsigned SrcReg = MI->getOperand(1).getReg();
+    SmallVector<std::pair<unsigned, unsigned>, 2> UsedRegsInCopy;
+    SmallVector<unsigned, 2> DefedRegsInCopy;
----------------
Any particular reason why we need to store both the register and the operand idx?


================
Comment at: lib/CodeGen/MachineSink.cpp:1091
+      // other targets.
+    } else if (MO.isUse()) {
+      if (ModifiedRegs[Reg]) {
----------------
junbuml wrote:
> I keep isUse() here with FIXME since I'm not perfectly sure if skipping the internal read is safe in all other targets. 
Ok, sounds good to me.


https://reviews.llvm.org/D44958