[PATCH] D53992: [DebugInfo] Correctly sink DBG_VALUEs in postra-machine-sink

Thu Nov 1 12:45:51 PDT 2018

jmorse created this revision.
jmorse added reviewers: aprantl, vsk, mattd, bjope.
Herald added subscribers: llvm-commits, JDevlieghere.

As reported in PR38952 [0] the postRA machine-code sinking of copies relies on relevant DBG_VALUEs always immediately following the instruction that's being sunk. The PR has a counterexample to this, and DBG_VALUE location does not seem guaranteed after the register allocator runs.

Because postra-machine-sink walks backwards through each BB looking for copies to sink, it will walk past any DBG_VALUE that refers to a copy it subsequently sinks. This means we don't need to scan the whole block looking for DBG_VALUEs, only record which ones have already been seen.

This patch collects seen DBG_VALUEs into a PhysRegister : DBG_VALUE multimap, and hands a vector of relevant DBG_VALUEs down to performSink if a copy gets sunk. Multimap is expensive, but there are no guarantees AFAIK about what order we'll encounter DBG_VALUEs in or what registers they may refer to, so I can't see what else to use.

Highly relevant to this patch is the discussion in [1] regarding the validity of sinking DBG_VALUEs past other DBG_VALUEs of the same variable, re-ordering the appearance of values for the user. My summary would be "yes that's a problem, but it's currently an acceptable tradeoff" (YMMV).

Building debug clang+llvm using another clang+llvm with and without this patch, completes in the same amount of time (give or take a second).

[0] https://bugs.llvm.org/show_bug.cgi?id=38952
[1] https://reviews.llvm.org/D45637

Repository:
  rL LLVM

https://reviews.llvm.org/D53992

Files:
  lib/CodeGen/MachineSink.cpp
  test/CodeGen/X86/pr38952.mir

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D53992.172198.patch
Type: text/x-patch
Size: 7756 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20181101/0cfe6cf7/attachment.bin>