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

[Jeremy Morse via Phabricator via llvm-commits]

jmorse added a comment.

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

> I believe this is already covered somewhat by /test/Transforms/LoopStrengthReduce/dbg-preserve-2.ll.

I think the usual rules apply: if there's an observable change in output from the compiler then there needs to be a test that fails if the modification isn't applied. As that test isn't currently failing, it doesn't cover the behaviour this patch adds. Otherwise LGTM



================
Comment at: llvm/lib/Transforms/Scalar/LoopStrengthReduce.cpp:6189
       }
+      // LSR may have updated the expression, so restore to the expression
+      // that corresponds to te cached SCEV.
----------------
"updated" is a little vague, could we go for "attempted to salvage" or otherwise indicate that normal debug-info mechanisms might have duly modified the expression.


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

https://reviews.llvm.org/D107016