[PATCH] D148536: [Assignment Tracking] Fix fragment error for some DSE-shortened stores

Orlando Cazalet-Hyams via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Apr 18 08:52:29 PDT 2023


Orlando updated this revision to Diff 514657.
Orlando marked 3 inline comments as done.
Orlando added a comment.

In D148536#4277449 <https://reviews.llvm.org/D148536#4277449>, @jmorse wrote:

> I guess we're finding all the twisty turney passages (all different) through the compiler that generate uncertainty over variable locations. On the one hand this is exactly what we want to be finding with assignment tracking, on the other hand it sucks that we're now needing to preserve so much detail. Discussed offline -- possibly the correct path is to relax the verifier requirements and just accept some imprecision (but not inaccuracy) in these intrinsics, it depends on whether this is the last corner case.
>
> For this particular patch I think it's hard to track exactly what's going on given that there are now a bunch of different fragments flying around. Refactoring into some utilities in DebugInfoMetadata.cpp would be preferred to reduce the amount we're expanding the DSE file by, plus some ascii-art diagrams of exactly what fragments we're working with and manipulating. It's going to be hard to maintain in the future unless it's very plain. IMO as the terms "variable space" and "dest space" don't show up elsewhere in assignment tracking some more standardised nomenclature should be found.

Thank you for reviewing this. I've moved the functions to more appropriate places outside of DeadStoreElimination.cpp, addressed the nits, and added a large comment to the body of `calculateFragmentIntersect`. wdyt? I could probably chop the first paragraphs before `# Example 1` out, but the additional context may be useful?


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

https://reviews.llvm.org/D148536

Files:
  llvm/include/llvm/IR/DebugInfo.h
  llvm/include/llvm/IR/DebugInfoMetadata.h
  llvm/include/llvm/IR/IntrinsicInst.h
  llvm/lib/IR/DebugInfo.cpp
  llvm/lib/Transforms/Scalar/DeadStoreElimination.cpp
  llvm/lib/Transforms/Scalar/SROA.cpp
  llvm/test/DebugInfo/Generic/assignment-tracking/dse/dse-after-memcpyopt-merge.ll
  llvm/test/DebugInfo/Generic/assignment-tracking/dse/shorten-offset.ll
  llvm/test/DebugInfo/Generic/assignment-tracking/dse/shorten.ll

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D148536.514657.patch
Type: text/x-patch
Size: 33066 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20230418/06d521d6/attachment.bin>


More information about the llvm-commits mailing list