[PATCH] D44312: Retain both sets of debug intrinsics in HoistThenElseCodeToIf (fixes PR 36410)

[Ulrich Weigand via Phabricator via llvm-commits]

uweigand added a comment.

In https://reviews.llvm.org/D44312#1033382, @efriedma wrote:

> I think you get approximately the right behavior if you just unconditionally ignore the debug info intrinsics (and never hoist them).  Of course, the debug info gets dropped if the terminator is hoisted, but that seems okay; it's hard to handle that well in general anyway.


This is of course an option, but it would break an existing test case (test/Transforms/SimplifyCFG/hoist-dbgvalue.ll) that was added by dpatel in 2011 to specifically verify that the following case is handled: the terminator is hoisted, but both "then" and "else" branch contain a dbg.value statement declaring the same value for the same variable -- in this case, the test case wants to verify that we still retain a dbg.value for that value in that variable.

The problem is that the code that does this hoisting of dbg.value statements for the same value/variable can still generate invalid results due to a mismatching !dbg location metadata field, that's the root cause of PR 36410.   So far, a number of different potential solutions have been proposed:

- make sure the merged !dbg location matches the debug value -- this is https://reviews.llvm.org/D43687, but that hasn't been approved yet since it may cause creation of "strange" locations (e.g. using inlined-to fields pointing to places where there is no inlining in the source code)
- do not merge debug values, but hoist them both -- this is the current Phabricator, which has the problem you pointed out
- do not hoist debug values at all -- your latest suggestion, but this would break the existing test case

I'm a bit unsure now what the best way to proceed is at this point.  My main goal right now is simply to fix the internal compiler errors in PR 36410 ...

Would it be an option to only hoist debug info instructions where both variable and value match (just like the existing code), but then not attempt to merge the !dbg locations, but instead just hoist both copies of the original instructions with both the original locations (like in this patch)?


Repository:
  rL LLVM

https://reviews.llvm.org/D44312