[PATCH] D133315: [Assignment Tracking][20/*] Account for assignment tracking in DSE

Orlando Cazalet-Hyams via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Sep 13 07:01:15 PDT 2022


Orlando updated this revision to Diff 459741.
Orlando marked an inline comment as done.
Orlando edited the summary of this revision.
Orlando added a comment.

In D133315#3780235 <https://reviews.llvm.org/D133315#3780235>, @jmorse wrote:

> If I understand the dse-after-memcpyopt-merge.ll test, the stores after the memset get deleted, because they're effectively done by the memset. However: don't we get the right behaviour with no modifications? There's an assignment of zero to those two fields, and the dbg.assigns record those facts even after the stores are deleted. The location list computed at the end of compilation can rightly say those fields are zero, no?
>
> Same with the second test -- the optimisation is shortening the amount of storing happening, but it isn't optimising away the assignment to those fields, just re-implementing them.
>
> To put it another way -- why is it necessary for those fields to be undef after the shortening of the overlapping stores?

Good question (another! I very much appreciate this level of review, thank you!). The value component shouldn't need to be undef, but this one is slightly odd because I chose to link the inserted undef dbg.assign to the memset. I think my original intention was to make it "obvious" that the dbg.assign marked the assignment as not-in-memory despite retaining the link.  However, a) I'm not convinced that retaining the link is the right thing to do, and b) an undef address component should be enough to signal that the memory location is stale for this fragment if it were.

Yes, thinking about it more, I think we should not retain the `DIAssignID` link and the value component shouldn't be `undef`'d. Updated.


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

https://reviews.llvm.org/D133315

Files:
  llvm/lib/Transforms/Scalar/DeadStoreElimination.cpp
  llvm/test/DebugInfo/Generic/assignment-tracking/dse/dse-after-memcpyopt-merge.ll
  llvm/test/DebugInfo/Generic/assignment-tracking/dse/shorten.ll

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D133315.459741.patch
Type: text/x-patch
Size: 21251 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20220913/fec61ff7/attachment.bin>


More information about the llvm-commits mailing list