[PATCH] D88898: [DebugInfo][InstrRef][4/4] Support using DBG_INSTR_REF through all* backend passes

[Jeremy Morse via Phabricator via llvm-commits]

jmorse added inline comments.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp:5581
+  if (Op->isReg())
+    NewMI = MakeVRegDbgValue(Op->getReg(), Expr, IsDbgDeclare);
+  else
----------------
Orlando wrote:
> Shouldn't the `MakeVRegDbgValue` parameter `Indirect` here be `IsIndirect` rather than `IsDbgDeclare`, or are `IsIndirect` and  `IsDbgDeclare` equivalent at this point? I'm just looking at the removed code in the diff so could be missing something.
I think this hinged on an assumption that I made in the past, that DbgDeclares only ever pointed at memory operands or something. That assumption turned out to be false, there was an assertion left hanging that D101594 cleared up.

Switched to passing IsIndirect in for all these things.


================
Comment at: llvm/lib/Target/X86/X86InstrInfo.cpp:4080
+    // Mutating this instruction invalidates any debug data associated with it.
+    CmpInstr.setDebugInstrNum(0);
     // Fall through to optimize Cmp if Cmp is CMPrr or CMPri.
----------------
Orlando wrote:
> Is this covered by any of the tests?
Added x86-drop-compare-inst.mir to cover this.


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

https://reviews.llvm.org/D88898