[PATCH] D64595: [Debuginfo][SROA] Need to handle dbg.value in SROA pass.

[Jeremy Morse via Phabricator via llvm-commits]

jmorse added a comment.

> Probably, it is OK if dbg.value was not originally _supposed_ to be a dbg.declare but was converted in this place.

Ah, this is core of the matter I think. Converting a dbg.value into a dbg.declare implicitly extends the duration ("lifetime" in the docs [0]) of the variable location from "until the next dbg.value" to "the entire scope". Consider what happens if we make the modifications here [1] to your test case. Imagine an IR producer that wants to temporarily bind some variable to a field of 'result' (the dbg.value added with DW_OP_deref), and later assigns '1' to the variable with the second dbg.value, to cover the rest of the function.

(Note that I've also disabled lines 4394->4396 of SROA.cpp, which deletes newly created dbg.declares if there are any more to create).

With this patch (and mod), the first added dbg.value intrinsic becomes a dbg.declare, which extends the lifetime of the location to overlap the subsequent dbg.value(1,...). Mem2reg will later add dbg.value intrinsics where there were stores, giving us this:

  define dso_local i32 @_Z3barv() #0 !dbg !7 {
  entry:
    %call = call i32 @_Z3foov(), !dbg !23
    call void @llvm.dbg.value(metadata i64 1, metadata !21, metadata !DIExpression()), !dbg !24
    call void @llvm.dbg.value(metadata i32 %call, metadata !12, metadata !DIExpression()), !dbg !25
    call void @llvm.dbg.value(metadata i32 %call, metadata !21, metadata !DIExpression(DW_OP_LLVM_fragment, 0, 32)), !dbg !25
    call void @llvm.dbg.value(metadata i32 %call, metadata !26, metadata !DIExpression(DW_OP_LLVM_fragment, 0, 32)), !dbg !30
    %cmp.i = icmp eq i32 %call, 0, !dbg !33
    br i1 %cmp.i, label %if.then, label %if.end, !dbg !34

(!21 = the variable I added, !12 = "result", !26 = "this").

In this circumstance, the variable I added now points somewhere else (which might be fixable). However the duration of the variable location has changed too: the variable was supposed to take on the value "1" for the rest of the function, but instead it gets some other location. I really doubt this is fixable -- both dbg.declare and dbg.value are working as intented, it's converting one to another that changes the lifetimes.

Note that clang probably wouldn't produce that input code ever; however LLVM has to support arbitary IR producers. I would also expect the first dbg.value I added to be dropped by LLVM at some point anyway: baking lots of logic into a DIExpression makes it harder for subsequent optimisations to preserve dbg.values.

[0] https://llvm.org/docs/SourceLevelDebugging.html#debugger-intrinsic-functions
[1] https://reviews.llvm.org/P8157


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

https://reviews.llvm.org/D64595