[PATCH] D73478: [DebugInfo] replaceDbgUsesWithUndef before removing single store alloca

[David Blaikie via Phabricator via llvm-commits]

dblaikie added a comment.

In D73478#1844009 <https://reviews.llvm.org/D73478#1844009>, @Orlando wrote:

> Hi @dblaikie, thanks for your comments.
>
> > At least in this example, while it has an effect on the generated IR, it doesn't seem to have an effect on the resulting object file
>
> Now you mention it I think a couple of these I have lined up should not ever affect the object file, but do, because of this:
>
> Good: `clang -O2 -g -emit-llvm ex.c -o -` adds all DILocalVariables to the DISubprogram retainedNodes list (even when the DILocalVariable still has other uses).
>  Bad: `clang ex.c -O0 -g -emit-llvm  -Xclang -disable-O0-optnone -o - | opt -O2 -S ` does not on trunk, but does with my changes because the undef-instead-of-dropped dbg.value uses the DILocalVariable metadata.
>
> At a glance it looks like running at >O0 will add all DILocalVariables to a DISubprogram retainedNode list, and running at O0 does not.
>
> Not sure if this is a bug or something I just wasn't aware of. I was compiling all of these test cases with the 'Bad' build line above, so any  dropped dbg.values which were the last users of the DILocalVariable metadata caused the local variable to be omitted from the final dwarf.


Oh, I noticed you'd generated your tests that way and was going to suggest using -O2 -Xclang -disable-llvm-passes instead, but didn't think it'd matter - but you're quite right, that Clang/LLVM does behave differently, by adding the "variables:" list to the DISubprogram when optimizing. This is intentional, to ensure local variables (& especially parameters) are not lost due to optimizations.

So how did you come to be trying to fix these bugs, if they seem to be not observable through the normal usage of Clang/LLVM? Perhaps we shouldn't fix them if there's no way for them to be problematic? (though I suspect with some work you might be able to find user-visible bugs at these sources like the sort of path I described (where the transformation is currently removing dbg value intrinsics - in these cases where none remain it's probably benign, but if there were other dbg values in other codepaths that might otherwise be clobbered by the undef, without it they might spill over & start producing misleading locations in unrelated pieces of code)


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

https://reviews.llvm.org/D73478