Remove debug instructions on dead temporaries in CopyForwarding

[meg-gupta]

In CopyForwarding, we forward copy into a dead temp.
Example:
copy_addr %src to [init] %temp
copy_addr %temp [take] to %dest
becomes
copy_addr %src to %dest

Any debug_value_addr instructions on the dead temp between the source to temp copy and temp to dest copy refers to uninitialized memory. This PR deletes these debug_value_addr instructions.

[meg-gupta]

I found this issue while doing the OSSA conversion for CopyForwarding. In OSSA MemoryLifetime verifier asserts on these debug_value_addr instructions on dead temp.

[atrick]

This original code here is unnecessarily confusing because the comments talk about a "source" and "temp" address, but the users of the "temp" are captured in the member variables called SrcUserInsts and SrcDebugValueInsts (feel free to clarify that :)

I think it would be slightly more consistent to rely on the existing CopySrcUserVisitor, which is guaranteed to account for all the uses of the deleted temp. I believe it puts all the debug_value users in SrcDebugValueInsts.

The existing instruction scan already identifies all the user within the deleted scope:

    if (SrcUserInsts.count(UserInst))
      return nullptr;

It's just missing this check:

    if (SrcDebugValueInsts.count(UserInst)) {
      deletedDebugValues.push_back(UserInst);
      continue;
    }

[meg-gupta]

@atrick Thanks, that is certainly more consistent. I have updated my changes.

[meg-gupta]

@swift-ci test

[swift-ci]

Build failed
Swift Test Linux Platform
Git Sha - f05262855033115ab4d4ffdb201b60bb00a36136

[swift-ci]

Build failed
Swift Test OS X Platform
Git Sha - f05262855033115ab4d4ffdb201b60bb00a36136

[atrick]

Thanks!

[gottesmm]

Great catch! This is exactly the type of stuff to look for as we port!