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

Wed Nov 18 06:33:19 PST 2020

bjope added inline comments.

================
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
----------------
bjope wrote:
> 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?
Looking at patch 2/3 it seems like InlineLowerDbgDeclare only handle the dbg.value that are adjacent to the call.

One could use an intermediate MaybeToBeRemoved map that is cleaned when finding a call, or appended to ToBeRemoved (and cleaned) when finding a non-dbg-intrinsic during the forward scan. That would pinpoint the algorithm to just keep dbg.value+deref just before a call. But maybe not worth the fuzz if these otherwise redundant dbg.value+deref instructions are rare.

Still, it might be nice to for example duplicate the dbg.value here and on line 31 to see that the backward scan still eliminates redundant dbg.value+deref when they are adjacent to each other (non non-dbg-intrinsic in between).

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

https://reviews.llvm.org/D91425