[PATCH] D91425: [DebugInfo] Skip dbg.value+derefs in RemoveRedundantDbgInstrs forward scan [3/3]

[Bjorn Pettersson via Phabricator via llvm-commits]

bjope added inline comments.


================
Comment at: llvm/lib/Transforms/Utils/BasicBlockUtils.cpp:419-421
+    // Check if this is a dbg.value+deref. Identical consecutive indirect
+    // locations may mark the assignment of different values to the
+    // location. In addition, InlineLowerDbgDeclare currently relies on finding
----------------
Not sure I understand exactly how/if this motivates why the dbg.value should be kept. If we first say that the variable is at address X, and then later say that the variable is at address X, the second statement seem redundant to me.


================
Comment at: llvm/test/DebugInfo/Generic/dont-remove-redundant-dbg-derefs.ll:29
+  store i32 %param, i32* %param.addr, align 4
+  call void @llvm.dbg.value(metadata i32* %param.addr, metadata !12, metadata !DIExpression(DW_OP_deref)), !dbg !13
+  call void @use(i32* nonnull %param.addr), !dbg !18
----------------
So if you do this ten times in a row (couldn't that be the case if for example unrolling a loop fully?) you would keep all of them as well.
(well I guess the backward scan would clean it up if they really are consecutive)

An alternative would be to filter out all entries with a deref from the VariableMap when finding a call. Or is it still important that the dbg.value with a deref is just before the call? Then, how is it guaranteed that no other pass is inserting something between the dbg.value and the call in such sitautions?


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

https://reviews.llvm.org/D91425