[PATCH] D98644: [DebugInfo] Fix incorrect handling of variadic salvaging in Loop Strength Reduce

[Markus Lavin via Phabricator via llvm-commits]

markus accepted this revision.
markus added inline comments.


================
Comment at: llvm/lib/Transforms/Scalar/LoopStrengthReduce.cpp:5892
+    DVI->setRawLocation(DbgValueToLocation[DVI].second);
+    DVI->setExpression(DbgDIExpr);
+    assert(DVI->isUndef() && "dbg.value with non-undef location should not "
----------------
StephenTozer wrote:
> markus wrote:
> > Is it really safe to always refresh from the map here? Isn't there a risk that we restore an old pre-LSR location that is no longer valid and if it turns out that we cannot find an, non-deleted, equivalent value in the loop below then we leave the `dbg.value` with incorrect information and not just undef?
> I believe so: the "raw location" that is the first argument of a debug variable intrinsic will be either a `DIArgList` or a `ValueAsMetadata`, rather than a direct `Value` pointer. Both of these classes have handlers for RAUW and value deletion, and so should not point to an invalid location. The only case that could slip past here, I think, is a potential "use before def" situation if a value referred to by the old location metadata is still valid but has sunk past the debug intrinsic; I can put a guard against that to be on the safe side.
Ok, sounds good. Feel free to add the guard but I am fine either way.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98644