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

[Chris Jackson via Phabricator via llvm-commits]

chrisjackson added a comment.

In D107016#2957650 <https://reviews.llvm.org/D107016#2957650>, @StephenTozer wrote:

> Hmm, actually I hate to suddenly jump in with a broader objection (especially after I already reviewed this earlier), but is this change needed? I think the reason that the cached DIExpression restoration is only needed for arglists is because it will cause an error if we have a mismatch between the number of ArgList elements and the number of elements referenced in the DIExpression; we don't (I think) otherwise care about whether the expression is restored or not, because it should be undef regardless (otherwise it wouldn't have been salvaged).

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.


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

https://reviews.llvm.org/D107016