[PATCH] D22466: Avoid false dependencies of undef machine operands

[Matthias Braun via llvm-commits]

MatzeB added inline comments.


================
Comment at: llvm/trunk/lib/CodeGen/ExecutionDepsFix.cpp:492-494
+  // Get the undef operand's register class
+  const TargetRegisterClass *OpRC =
+      TII->getRegClass(MI->getDesc(), OpIdx, TRI, *MF);
----------------
getRegClass() can return nullptr if there is not register class declared/defined in MCInstrDesc (happens mostly on generic instructions like COPY or INLINEASM). The code looks like it would crash in this case.


================
Comment at: llvm/trunk/lib/CodeGen/ExecutionDepsFix.cpp:499
+  for (MachineOperand &CurrMO : MI->operands()) {
+    if (!CurrMO.isReg() || CurrMO.isDef() || CurrMO.isUndef() ||
+        !OpRC->contains(CurrMO.getReg()))
----------------
I would use !CurrMO.readsReg() instead of isUndef(), that way bundle internal reads are ignored as well.


================
Comment at: llvm/trunk/lib/CodeGen/ExecutionDepsFix.cpp:512
+  unsigned MaxClearanceReg = OriginalReg;
+  for (unsigned rx = 0; rx < OpRC->getNumRegs(); ++rx) {
+    unsigned Clearance = CurInstr - LiveRegs[rx].Def;
----------------
This looks bogus if OpRC is ever != ExeDepsFix::RC. Maybe you shouldn't even query OpRC and use ExeDepsFix::RC? Or can there be situations in which an operand has a restricted register class?


================
Comment at: llvm/trunk/lib/CodeGen/ExecutionDepsFix.cpp:523-525
+  // Update the operand if we found a register with better clearance.
+  if (MaxClearanceReg != OriginalReg)
+    MO.setReg(MaxClearanceReg);
----------------
If multiple operands use the same register, they all need to be updated at the same time (MachineOperand documents that even for undef inputs with the same register read the same value).



Repository:
  rL LLVM

https://reviews.llvm.org/D22466