[PATCH] D86812: [DebugInstrRef][1/3] Track PHI values through register allocation

[Djordje Todorovic via Phabricator via llvm-commits]

djtodoro added a comment.

LGTM, some nits inline. Thanks.



================
Comment at: llvm/include/llvm/CodeGen/MachineFunction.h:468
+  /// during register allocation. See DebugPHIRegallocPos.
+  std::map<unsigned, DebugPHIRegallocPos> DebugPHIPositions;
+
----------------
Do we need the `std::map` `sortness`? Can we use `llvm::DenseMap` instead?


================
Comment at: llvm/lib/CodeGen/LiveDebugVariables.cpp:1226
+#ifndef NDEBUG
+    Register r = PHIIt->second.Reg;
+    assert(r == OldReg);
----------------
should be capital letter


================
Comment at: llvm/lib/CodeGen/LiveDebugVariables.cpp:1587-1589
+    } else {
+      // There is no mapping for this value ID, it's gone. Create no DBG_PHI.
+    }
----------------
I guess we do not need the `else` here.


================
Comment at: llvm/lib/CodeGen/PHIElimination.cpp:323-324
+    unsigned ID = MPhi->peekDebugInstrNum();
+    auto p = MachineFunction::DebugPHIRegallocPos(&MBB, IncomingReg, 0);
+    auto res = MF->DebugPHIPositions.insert({ID, p});
+    assert(res.second);
----------------
these should be capital letters, I guess


================
Comment at: llvm/lib/CodeGen/PrologEpilogInserter.cpp:1243
+      } else if (MI.isDebugPHI()) {
+        continue; // allow stack ref to continue onwards
       }
----------------
dstenb wrote:
> Nit: Capital letter and period.
the comment should go above the line


================
Comment at: llvm/test/DebugInfo/MIR/InstrRef/phi-regallocd-to-stack.mir:21-27
+  ; Function Attrs: nounwind readnone speculatable willreturn
+  declare void @llvm.dbg.declare(metadata, metadata, metadata) #0
+  
+  declare dso_local i32 @ext(i32)
+  
+  ; Function Attrs: nounwind readnone speculatable willreturn
+  declare void @llvm.dbg.value(metadata, metadata, metadata) #0
----------------
unused, could be deleted


================
Comment at: llvm/test/DebugInfo/MIR/InstrRef/phi-through-regalloc.mir:50-51
+  
+  ; Function Attrs: nounwind readnone speculatable willreturn
+  declare void @llvm.dbg.declare(metadata, metadata, metadata) #0
+  
----------------
unused, could be deleted


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

https://reviews.llvm.org/D86812