[PATCH] D18427: CodeGen: Add DetectDeadLanes pass.

[Quentin Colombet via llvm-commits]

qcolombet added inline comments.

================
Comment at: lib/CodeGen/DetectDeadLanes.cpp:149
@@ +148,3 @@
+  const MachineInstr &MI = *Def.getParent();
+  switch (MI.getOpcode()) {
+  case TargetOpcode::COPY:
----------------
I wouldn’t have expected overlaps, but you’re right this is not forbidden.
That being said, this means that we make an assumption on how the register sequence will be broken down and scheduled.

I would rather treat each operand independently and ignore the overlapping but print something in the debug output if the case happens.

================
Comment at: lib/CodeGen/DetectDeadLanes.cpp:253
@@ +252,3 @@
+      DefinedLanes &= ~TRI->getSubRegIndexLaneMask(SubIdx);
+    }
+    break;
----------------
I know it does not use any sub-register liveness.

What I am saying is without that pass, the coalescer might expose dead defs it was not expecting and cause later crashes.
I.e., if I understood correctly, this pass should always run to avoid potential crashes, right?

================
Comment at: lib/CodeGen/DetectDeadLanes.cpp:306
@@ +305,3 @@
+    if (lowersToCopies(DefMI)) {
+      // Start optimisatically with no used or defined lanes for copy
+      // instructions. The following dataflow analysis will add more bits.
----------------
We usually put the “noreg” case earlier.
In that case, I would have expected this check to be right after getReg().

That being said, I am fine with the current form, it is just unusual :).


Repository:
  rL LLVM

http://reviews.llvm.org/D18427