[PATCH] D55987: [CodeGen] Skip over dbg-instr in twoaddr pass

[Markus Lavin via Phabricator via llvm-commits]

markus added inline comments.


================
Comment at: lib/CodeGen/TwoAddressInstructionPass.cpp:932
   MachineBasicBlock::iterator End = AfterMI;
-  while (End->isCopy() &&
-         regOverlapsSet(Defs, End->getOperand(1).getReg(), TRI)) {
-    Defs.push_back(End->getOperand(0).getReg());
+  while (End->isDebugInstr() ||
+         (End->isCopy() &&
----------------
vsk wrote:
> aprantl wrote:
> > For IR we have an iterator that skips over debug intrinsics automatically. Does the same thing exist for MIR, too?
> Yes, I believe we have skipDebugInstructionsForward / skipDebugInstructionsBackward. It might be neat to use that here.
If we were to use skipDebugInstructionsForward then it seems that the loop would have to be written as follows to preserve the semantics (and handle multiple COPYs with DBG_VALUEs interspersed in various ways).

```
while (End != MBB->end()) {
  End = skipDebugInstructionsForward(End, MBB->end())
  if (End->isCopy() && regOverlapsSet(Defs, End->getOperand(1).getReg(), TRI)))
    Defs.push_back(End->getOperand(0).getReg());
  else
    break;
  ++End;
}
```
Would that be better?


================
Comment at: test/CodeGen/X86/twoaddr-dbg-value.mir:12
+    %4:gr32 = XOR32ri8 %1, 1, implicit-def dead $eflags
+    DBG_VALUE %4
+    %5:gr32 = COPY %4
----------------
vsk wrote:
> This is mostly an academic question, but: is this a valid DBG_VALUE? I'm a little surprised this parses (I'd expect the instruction to have more operands), but I like the simplicity of it.
It is probably not valid but does seem to be sufficient for the purpose of this test (where the presence of a DBG_VALUE is the only relevant thing and not the contents of it). I would be happy to make it more valid though if that is just a matter of adding the additional operands.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D55987