[PATCH] D45637: [DebugInfo] Ignore DBG_VALUE instructions in PostRA Machine Sink

[Matt Davis via Phabricator via llvm-commits]

mattd added a comment.

@bjope , Thanks for your review and example!   I agree that any changes to improve performSink/collectDebugValues should probably be in another patch.

I agree, after spending some time with your hypothetical case, I can imagine there are some other cases that might result in old/intermediate values in the user's variable, at least for a short period of time.

In https://reviews.llvm.org/D45637#1129560, @bjope wrote:

>   ...
>   renamable $eax = COPY $edi
>   DBG_VALUE debug-use $eax, debug-use $noreg, !14, !DIExpression(), debug-location !16
>   $esi = ADD $edi, 7
>   DBG_VALUE debug-use $esi, debug-use $noreg, !14, !DIExpression(), debug-location !17
>   ...
>   


Excuse me if I am getting this wrong, the following is my interpretation of your example.   Let's assume this example lives in bb0.   In the example, sticking with x86_64, the ADD (or ADD32ri) instruction expects tied physical registers, something like `$edi = ADD32ri $edi, 7`  However, that case would prevent the COPY from sinking, I'm guessing due to a register def.  If we were to make this `$esi = ADD32ri $esi, 7`, that would allow for the COPY to be sunk.  Variable !14 would truly be $esi +7 at that program point in bb0, because the DBG_VALUE you introduced will associate $esi as the value for !14.  The sunken copy, presumably in bb1, would be the value copied from $edi, and probably the value that a user would expect.  Single stepping from the ADD to the sunken COPY would show !14 as being an intermediate or 'old' value until the sunken copy is reached.  This doesn't seem wrong to me.  It just could present unexpected values to the user, when inspecting !14 between the ADD and sunken COPY. Perhaps, if we were to delete just the DBG_VALUE associated to the  ADD  in bb0 ,  perhaps that would encourage a debugger to present 'optimized-out' in this case. I agree with you, any refinements to performSink or collectDebugValues should probably be for another patch.  Thanks again for your review.


https://reviews.llvm.org/D45637