[PATCH] D22858: Fix DbgValue handling in SelectionDAG.

Nirav Davé via llvm-commits llvm-commits at lists.llvm.org
Thu Jul 28 06:23:56 PDT 2016


i strongly believe (but have not checked because I had not been able to
reproduce the bug locally and have been relying on Ismall running the test)
that the sole part of the fix necessary is the change of  location
TransferDbgValues in ReplaceAllUsesWith to be consistent with the other N
instances of ReplaceAllUses and do the transfer before changing the SDUses
and doing CSE. This was clearly wrong the dest SDNode may be merged in CSE
and the old pointer may point to free space.

Now that I (hopefully) can reproduce this bug with pr28749, I can verify
that the second half of the patch is not necessary for correctness and we
can potentially split that off.

The second half of the patch regards how we should be Transferring debug
Values.

Previous to r273585 when replacing a Node/Value with another we would
mostly copy debug values, but not for all replacements which resulted in
DebugValues being dropped when doing some optimization as the old node was
optimized away and the new node didn't get a cloned version. It appears to
compensate for this dropping we would copy all DebugValues associated with
the entire SDNode when transferring a SDValue. Since this is debugging
information lost during an optimization slightly off debug info isn't
obviously wrong.

r273585 changed it so we always clone exactly the DebugValues associated
with a SDValue we replaced in the SelectionDAG. The original DebugValues
stays associated with the original SDValue. If later transformations merge
this node with another it is possible that the original value will remain
to the end and we will see multiple instances of the original debug values.
I think this is not what we want to do as intuitively all paths associated
with the DebugValue would have been transferred and any future uses would
come from a different path in the original source. This patch invalidates
the original debug values preventing the multiple instances.

-Nirav

On Wed, Jul 27, 2016 at 5:43 PM, Eric Christopher <echristo at gmail.com>
wrote:

> echristo added a comment.
>
> Can you elaborate on what's going on here a bit more and why this is the
> correct fix? We've poked at this a few times lately so it's not clear.
>
> Thanks!
>
> -eric
>
>
> https://reviews.llvm.org/D22858
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160728/1d518057/attachment.html>


More information about the llvm-commits mailing list