[PATCH] D22466: Avoid false dependencies of undef machine operands + fix bug in clearance calculation

[Michael Kuperstein via llvm-commits]

mkuper added inline comments.

================
Comment at: lib/CodeGen/ExecutionDepsFix.cpp:486
@@ +485,3 @@
+
+  // Update only undef operands that are mapped to one register.
+  if (AliasMap[OriginalReg].size() != 1)
----------------
I'm a bit confused about this condition. Could you explain?

================
Comment at: lib/CodeGen/ExecutionDepsFix.cpp:492
@@ +491,3 @@
+  // behind it.
+  for (unsigned i = 0; i < MI->getNumOperands(); ++i) {
+    MachineOperand &CurrMO = MI->getOperand(i);
----------------
Why not a range for over MI->operands()?

================
Comment at: lib/CodeGen/ExecutionDepsFix.cpp:503
@@ +502,3 @@
+
+  // Go over all registers in the register class and find the register with the
+  // max clearance.
----------------
This happens only when the real input operand is in memory, right?
If so, is this better than undoing the load folding, and using the loaded register as undef? Or does that depend on the clearance / being able to xor the register? 

(This patch looks like a a net improvement regardless, I'm wondering about a possible TODO/follow-up patch)

================
Comment at: lib/CodeGen/ExecutionDepsFix.cpp:511
@@ +510,3 @@
+      continue;
+    MaxClearance = Clearance;
+    MaxClearanceReg = RC->getRegister(rx);
----------------
Can we break if we've passed the "minimum required clearance" (that is, Pref)?

================
Comment at: lib/CodeGen/ExecutionDepsFix.cpp:523
@@ -522,3 +567,1 @@
       continue;
-    if (MO.isImplicit())
-      break;
----------------
myatsina wrote:
> This is a fix to bug - the clearance calculation did not take into account registers defined as outputs or clobbers in inline assembly machine instructions because these register defs are implicit.
Any reason not to commit this separately?


Repository:
  rL LLVM

https://reviews.llvm.org/D22466