[PATCH] D40042: Make salvageDebugInfo of casts work for dbg.declare and dbg.addr

[Vedant Kumar via Phabricator via llvm-commits]

vsk added inline comments.


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineCasts.cpp:150
+  // soon.
+  if (ValueAsMetadata *MDUse = ValueAsMetadata::getIfExists(&AI)) {
+    LLVMContext &Ctx = New->getContext();
----------------
rnk wrote:
> vsk wrote:
> > rnk wrote:
> > > aprantl wrote:
> > > > are there metadata users besides debug info? Would findDbgValues() be more readable?
> > > I can't think of any non-debug info uses. I did it this way because I wanted it to handle dbg.value, dbg.declare, and dbg.addr uniformly. This matter in situations where the value of a variable is the address of a variable (my favorite test case):
> > >   struct Foo PromoteMe = *p;
> > >   struct Foo *addr = &PromoteMe; // dbg.value(&PromoteMe)
> > > 
> > > An alternative way of looking at this bug is that it is a failure of salvageDebugInfo to handle dbg.declare. Actually, I wonder if I can update its use of findDbgValues to use this pattern and see if that fixes the bug more generally.
> > In this case, how would salvageDebugInfo() know to attach the debug info from the old alloca to the new one? Would you pass a 'destination' Instruction to salvageDebugInfo?
> Without this code, the `tmpcast` instruction below is used as the replacement. Later, instcombine combines that cast with another cast, and we lose the dbg.declare. If we can make salvageDebugInfo work on deleted casts, we can fix this bug more generally.
Gotcha, thanks for explaining. I see how this is hooked up via eraseInstFromFunction() now.


https://reviews.llvm.org/D40042