[PATCH] D133294: [Assignment Tracking][11/*] Update RemoveRedundantDbgInstrs

[Jeremy Morse via Phabricator via llvm-commits]

jmorse added a comment.

If I understand the logic in the forward scan correctly, whenever you see a linked dbg.assign, you will _always_ keep the next debug intrinsic for that variable, because the expression is nullptr. The next debug intrinsic then resets the map, so if it's a dbg-value-like intrinsic, subsequent dbg-value-like intrinsics will be removed as redundant.

I can sort of see why this is necessary (moving a variable from "linked" to "not linked" is a meaningful transition that we shouldn't remove), is there a definite use case for it, and is it covered by the test? (I don't see it being covered by the test, but I might not have read it correctly).

As annoying as this is: the codepaths using SeenDefForAggregate should be tested by the test too, it'll make it easier for me to grasp what's going on.



================
Comment at: llvm/test/DebugInfo/Generic/assignment-tracking/remove-redundant.ll:17-18
+
+;; Forward scan: dbg.assign for Local unlinked with undef value component, seen
+;; before any non-undefs; delete it.
+; CHECK-NEXT: @step()
----------------
... in the entry block, being the important feature, no?


================
Comment at: llvm/test/DebugInfo/Generic/assignment-tracking/remove-redundant.ll:50-54
+;; Forward scan: Check that dbg.assign that looks like a dbg.value that is made
+;; redundant by the previous dbg.assign is removed.
+;; CHECK-NEXT: @step()
+  call void @llvm.dbg.assign(metadata i32 3, metadata !11, metadata !DIExpression(), metadata !15, metadata i32* undef, metadata !DIExpression()), !dbg !14
+  call void @step()
----------------
Deliberately identical to lines 44 -> 48?


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

https://reviews.llvm.org/D133294