[PATCH] D22858: Fix DbgValue handling in SelectionDAG.

Nirav Dave via llvm-commits llvm-commits at lists.llvm.org
Thu Jul 28 08:40:30 PDT 2016


niravd added a comment.

Apparently my email didn't get recorded here so I'm reproducing here:

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.


https://reviews.llvm.org/D22858





More information about the llvm-commits mailing list