[PATCH] D73168: Improvements to call site register worklist

Fri Jan 24 07:26:23 PST 2020

dstenb added inline comments.

================
Comment at: llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp:574
+  // register.
+  using FwdRegWorklist = DenseMap<unsigned, SmallVector<unsigned, 2>>;
+
----------------
vsk wrote:
> 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.
Good catch! As I've understood it the iteration order for a non-pointer type holding DenseMap is deterministic for a single binary? However, I guess it's not free from implementation-defined behavior (e.g. the hash function is dependent on the width of `unsigned`), so I guess there's no guarantee that the iteration order is deterministic between binaries built with different compilers. I made it a MapVector.

================
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
----------------
vsk wrote:
> 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.
Oh, right!

I switched to implicit-check-not in both tests.

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

https://reviews.llvm.org/D73168