[PATCH] D107016: [DebugInfo][LoopStrengthReduction] Ensure restoration of cached DIExpression when using SCEV-based salvaging

[Stephen Tozer via Phabricator via llvm-commits]

StephenTozer added a comment.

In D107016#2961940 <https://reviews.llvm.org/D107016#2961940>, @chrisjackson wrote:

> Let's assume a single location dbg.value and that LSR sets the location undef. When the dbg.value is cached it may have a DIExpression already. The scev-salvage method will write an expression that restores the location value. This new expression is combined with the old using prependOpCodes() in e.g. setFinalExpression(), in order to fully restore the value that the pre-LSR dbg.value computed. However, it is possible LSR may change the DIExpression as well as make the location undef. If this is the case, the DIExpression created by concatenating the expressions will not be correct.

Ah, so the second salvaging may succeed where the first has failed - in that case it makes sense, since the previous logic assumed that attempts to salvage had already failed. Personally, I think this is a fairly trivial fix, but I won't overrule others asking for a test; I wouldn't expect it to be too hard to write a test in that case though, as the only thing you'd need would be a single-value expression that fails the first salvage but succeeds the second, is that right?


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

https://reviews.llvm.org/D107016