[PATCH] D57962: [DebugInfo] PR40628: Don't salvage load operations

[Jeremy Morse via Phabricator via llvm-commits]

jmorse created this revision.
jmorse added reviewers: aprantl, bjope, dblaikie, probinson, vsk.
Herald added subscribers: llvm-commits, JDevlieghere.
Herald added a project: LLVM.

This patch disallows load insts from being salvaged. The reason why is PR40628 [0] -- LLVM is unable to determine where the validity of a debug expression containing DW_OP_deref ceases (i.e., where a store overwrites the intended data). It's also (I believe) unable to determine when hoisted/sunk memory instructions would invalidate a dbg.value. This has a large potential for misleading developers.

IMHO this isn't something that can be fixed without a lot of work, thus it's safer to turn off for now. If that's a contentious position though we can find a different way forwards. As mentioned in the PR, the effect on a clang-3.4 build is a 1.1% drop in variable location coverage and 1% drop in scope bytes coverage -- unfortunate, but IMHO not devastating. (I tend to put a lot more weight on avoiding inaccurate DebugInfo, as that diminishes trust in the accurate parts).

Summary of test changes:

- GVN test: -debugify option was added to test that instructions were salvaged in https://reviews.llvm.org/rL325063 . There's another file with -debugify checks added in that commit, so there's still coverage despite me deleting it in this test.
- DeadStoreElimination: tricky as memory operations are exactly what we're messing with. I've altered the test to have an intermediate addition that gets deleted by DSE as being redundant, and added a check that that's salvaged, which appears to have been the original aim of the test.
- InstCombine: this specifically tests for salvaging loads, so must be disabled. I've left a negative test there to fire if this feature is ever re-enabled in a safer condition.

The new test case is just a regression test so that there's a PR available if this behaviour comes back in the future.

[0] https://bugs.llvm.org/show_bug.cgi?id=40628


Repository:
  rL LLVM

https://reviews.llvm.org/D57962

Files:
  lib/Transforms/Utils/Local.cpp
  test/DebugInfo/Generic/pr40628.ll
  test/Transforms/DeadStoreElimination/debuginfo.ll
  test/Transforms/GVN/fence.ll
  test/Transforms/InstCombine/debuginfo-dce.ll

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D57962.185983.patch
Type: text/x-patch
Size: 7182 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20190208/79d84910/attachment.bin>