[PATCH] D73168: Improvements to call site register worklist

[Vedant Kumar via Phabricator via llvm-commits]

vsk added a comment.

This is looking really nice. Thanks!



================
Comment at: llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp:574
+  // register.
+  using FwdRegWorklist = DenseMap<unsigned, SmallVector<unsigned, 2>>;
+
----------------
I think this needs to be a MapVector. Otherwise, the iteration order in getForwardingRegsDefinedByMI will be non-deterministic. I.e. the insertion order into the `Defs` SmallSetVector will not be deterministic.


================
Comment at: llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp:650
+    for (auto ParamReg : ParamRegs)
+      I.first->second.push_back(ParamReg);
   };
----------------
Consider adding `assert(find(I.first->second, ParamReg) == nullptr && "Same parameter described twice by forwarding reg")` before pushing `ParamReg`?


================
Comment at: llvm/test/DebugInfo/MIR/X86/dbgcall-site-partial-describe.mir:76
+
+# CHECK-NOT: DW_TAG_GNU_call_site_parameter
+# CHECK: DW_TAG_GNU_call_site_parameter
----------------
I think the first CHECK-NOT for DW_TAG_GNU_call_site_parameter can safely be deleted. Or you could pass `-implicit-check-not=DW_TAG_GNU_call_site_parameter` to FileCheck and get rid of both CHECK-NOT's.


================
Comment at: llvm/test/DebugInfo/MIR/X86/dbgcall-site-reg-shuffle.mir:90
+# CHECK-NEXT: DW_AT_location	(DW_OP_reg4 RSI)
+# CHECK-NEXT: DW_AT_GNU_call_site_value	(DW_OP_GNU_entry_value(DW_OP_reg5 RDI))
+# CHECK-NOT: DW_TAG_GNU_call_site_parameter
----------------
Nice!


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

https://reviews.llvm.org/D73168