[PATCH] D58726: [DebugInfo][Docs] Explicitly document how dbg.value intrinsics are interpreted in optimized code

Jeremy Morse via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Mar 1 04:29:17 PST 2019


jmorse added inline comments.


================
Comment at: docs/SourceLevelDebugging.rst:405
+``llvm.dbg.value`` intrinsic is created for each assignment, recording the
+variables new location. Compared with the ``llvm.dbg.declare`` intrinsic:
+
----------------
probinson wrote:
> dbg.value can also record a (typically immediate) value, that is not an SSA register, so you'll need to do some rephrasing.
I've added an additional bullet point below, as this is also a difference with dbg.declare.


================
Comment at: docs/SourceLevelDebugging.rst:462
+
+  define i32 @foo(i32 %bar, i1 %cond) {
+    call @llvm.dbg.value(metadata i32 0, metadata !1, metadata !2)
----------------
bjope wrote:
> Did you intend to leave out dbg.value in this example? 
> Now it looks identical to the "Finally.." version below which is a little bit confusing.
Curses, you're correct, now updated,


================
Comment at: docs/SourceLevelDebugging.rst:487
+    %addoper = select i1 %cond, i32 11, i32 12
+    call @llvm.dbg.value(metadata i32 undef, metadata !1, metadata !2)
+    %toret = add i32 %bar, %addoper
----------------
bjope wrote:
> I do not think that we "must" introduce the undef in this example. We might prefer to do it (because it is a simple solution).
> But since there are no other context given here, and the value 0 is a valid state for the variable it isn't wrong to present the variable as having the value 0, up until it is incremented. Well, that is at least my opinion...
> 
> Although, if you for example add a function call like this:
> 
>   define i32 @foo(i32 %bar, i1 %cond) {
>     call @llvm.dbg.value(metadata i32 0, metadata !1, metadata !2)
>     br i1 %cond, label %truebr, label %falsebr
>   truebr:
>     %tval = add i32 %bar, 1
>     call @dbg.value(metadata i32 %tval, metadata !1, metadata !2)
>     call @gazonk()
>     br label %exit
>   falsebr:
>     %fval = add i32 %bar, 2
>     call @dbg.value(metadata i32 %fval, metadata !1, metadata !2)
>     call @gazonk()
>     br label %exit
>   exit:
>     %merge = phi [ %tval, %truebr ], [ %fval, %falsebr ]
>     call @dbg.value(metadata i32 %merge, metadata !1, metadata !2)
>     %toret = add i32 %merge, 10
>     call @dbg.value(metadata i32 %toret, metadata !1, metadata !2)
>     ret i32 %toret
>   }
> 
> then we should get
> 
>   define i32 @foo(i32 %bar, i1 %cond) {
>     call @llvm.dbg.value(metadata i32 0, metadata !1, metadata !2)
>     call @llvm.dbg.value(metadata i32 undef, metadata !1, metadata !2)
>     call @gazonk()
>     %addoper = select i1 %cond, i32 11, i32 12
>     %toret = add i32 %merge, %addoper
>     call @dbg.value(metadata i32 %toret, metadata !1, metadata !2)
>     ret i32 %toret
>   }
> 
> In my example I think it could be wise to mark `!1` as undef before the call to gazonk. The value 0 isn't a valid state for `!1` at the call. Still not sure if we must do it.
> 
> Although I guess the transformation in neither example is done in one single step. We will probably salvage the dbg.value inside the `truebr`and `falsebr`blocks along the way while sinking. So it might be likely that we end up with the undef as in your example. However, I can't see that the undef "must" be there given your example (at least it doesn't help me understand why/when it must be added).
> I do not think that we "must" introduce the undef in this example. We might prefer to do it (because it is a simple solution).
> [...]
>I can't see that the undef "must" be there given your example (at least it doesn't help me understand why/when it must be added).

Sure; in retrospect the example didn't demonstrate the motivating issue at hand, i.e. variable re-ordering. I've refreshed the example using yours as a base, and added an additional source variable (so we now have !1 and !3), the point of the example being that combinations of variable values that did not appear in the unoptimised program should not appear in the optimised one.

Thanks for the worked example, hopefully the new one is more convincing.


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

https://reviews.llvm.org/D58726





More information about the llvm-commits mailing list