[PATCH] D133310: [Assignment Tracking][15/*] Account for assignment tracking in simplifycfg

[Orlando Cazalet-Hyams via Phabricator via llvm-commits]

Orlando updated this revision to Diff 460381.
Orlando marked an inline comment as done.
Orlando added a comment.

> Soooo, for the TryToSimplifyUncondBranchFromEmptyBlock case I think this would expose the "LLVM can't represent things that happen on critical edges" issue [0], in that sinking the dbg.assigns out of a trivial block might put them in a block with other predecessors. IMO, it's preferable to maintain what dbg.values currently do, which is to vanish (sometimes causing wrong debug-info to be produced), because it's not a regression.
>
> Addressing this is a much larger issue; possibly we can deal with that in the future, but I'd suggest not as part of this patch series. Same with the "hoist all..." situation -- that might be an improvement, but we should start off with no regressions, then consider the improvements we can make.

In the past (prototype's past) this was a necessary change because dropping linked dbg.assign intrinsics led to incorrect locations. I think these changes have become less important now that we've introduced the "untagged stores still count as an assignment" rule. I have removed the changes - if this turns out to have caused trouble for some reason we can always re-evaluate.

> For the store-speculation that happens -- should there not be a call somewhere to merge the assign-IDs of these stores? (Possibly not)

Good spot - I didn't realise I had missed the test for this in the patch. Looking at it again, it turns out my comment (in the code) is misleading, as the other block's store isn't actually deleted (/merged) with the speculated store (it is deleted by DSE later). I've updated the code comment and added `speculated-store.ll`.

> Plus, is there a possibility / chance that
> ...

Yeah that sounds right. I've now guarded that with a check.


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

https://reviews.llvm.org/D133310

Files:
  llvm/lib/Transforms/Utils/SimplifyCFG.cpp
  llvm/test/DebugInfo/Generic/assignment-tracking/simplifycfg/empty-block.ll
  llvm/test/DebugInfo/Generic/assignment-tracking/simplifycfg/sink-erase.ll
  llvm/test/DebugInfo/Generic/assignment-tracking/simplifycfg/speculated-store.ll

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D133310.460381.patch
Type: text/x-patch
Size: 24260 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20220915/a85bbde5/attachment.bin>