[PATCH] D73159: ARM64: Debug info for structure argument missing DW_AT_location

[Vedant Kumar via Phabricator via llvm-commits]

vsk added inline comments.


================
Comment at: llvm/lib/CodeGen/GlobalISel/InstructionSelect.cpp:178
           MRI.replaceRegWith(DstReg, SrcReg);
-          MI.eraseFromParentAndMarkDBGValuesForRemoval();
+          MI.eraseFromParent();
         }
----------------
kamleshbhalui wrote:
> vsk wrote:
> > If MI is erased, why should its debug uses not be made undef? What really causes those debug uses to be updated correctly?
> since it is unoptimized it should not mark debug uses as undef.
> 
I'm not sure there's something special about unoptimized code here. GIsel seems to eliminate redundant copies even at -O0 (maybe to reduce compile time).

Oh, I think I see what's happening now. When GISel sees a redundant copy, it deletes it /after/ replacing vreg uses of `DstReg` with `SrcReg`, at which point valid debug uses of `SrcReg` are accidentally deleted.

Prior to the `replaceRegWith` call, are all debug uses of `DstReg` guaranteed to be dominated by the def of `SrcReg`? If so, could you leave a comment explaining why?

If not, for this particular test case, I'd expect that swapping the lines `MRI.replaceRegWith(DstReg, SrcReg);` and `MI.eraseFromParentAndMarkDBGValuesForRemoval();` would also cause the test to pass.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73159