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

[jmorse]

Summary

As discussed in D58453, the interpretation of dbg.value intrinsics and how they should be treated in optimized code isn't very well documented at the moment. This patch adds a section, ``Object lifetime in optimized code'' to the source level debugging documentation, that documents how such intrinsics are Supposed To (TM) be handled. This may be a can of worms because not everyone may agree that this is how they're interpreted.

Further documentation can be added, but what I wanted to get out there at this point is:

• dbg.values terminate earlier locations and their position in the instruction stream is significant, and
• that it's better to make a variable read ``optimized out'' than it is to present a state (set of variable valuations) that misrepresents the program,

which if we can agree on, will be Progress (TM).

Note that I haven't written any restructured text before :o

[aprantl]

Thanks! Small nitpicks inside.

docs/SourceLevelDebugging.rst

526

SSA values

528

values ...

533

for (any overlapping fragments of) the

544

``llvm.dbg.value``

551

and undermining their trust in the debugger.

554

ditto

555

... and let the debugger present ...

[probinson]

Some initial comments.

docs/SourceLevelDebugging.rst

519

This is the label for the following section, so it needs to stay after your new section.

530

dbg.value can also record a (typically immediate) value, that is not an SSA register, so you'll need to do some rephrasing.

533

I think "terminates the effect of any preceding dbg.value" might be better.

535

value or location

546

I agree with Adrian, "valuations" is not a term of art and I'd rather not see us introduce it. Maybe s/variable valuations/values of variables/g, or some other similar phrasing.

[bjope]

docs/SourceLevelDebugging.rst

587

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.

612

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 truebrand falsebrblocks 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).

[jmorse]

Apply review feedback, re-write the dbg.value undef example to better demonstrate the problem at hand.

docs/SourceLevelDebugging.rst

530

I've added an additional bullet point below, as this is also a difference with dbg.declare.

587

Curses, you're correct, now updated,

612

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.

[dstenb]

Some nitpicks.

docs/SourceLevelDebugging.rst

398

nit: variable's

408

nit: variable's

432

nit: Withholding

474

%merge -> %bar ?

[jmorse]

Sprinkle some possessive apostrophes, speling, incorrect variable names

[jmorse]

docs/SourceLevelDebugging.rst

474

Rats, I did that elsewhere, and had it duplicated in a dbg.value a few lines up too (now fixed).

[probinson]

A few wording notes. Haven't worked through the examples yet.

docs/SourceLevelDebugging.rst

409

"constant valued"? maybe "Operands can be constants, ..."?

433

valuations -> values

479

I'm happy to let you use dbg.value as a shorthand in most places, but when you specifically call them "intrinsics" I think you should use the full name.

[vsk]

+1, thanks for codifying all of this!

docs/SourceLevelDebugging.rst

403

variable's

413

Can this sentence ('Recording ... to a much greater extent') be a bit more direct? It might work to simply move it after the next paragraph, saying e.g. 'Care must be taken to update dbg.value intrinsics when: ...'.

[jmorse]

Incorporate further wording and structure feedback

[jmorse]

Re-think opening of paragraph on the risks involved when moving/altering code.

[jmorse]

docs/SourceLevelDebugging.rst

413

I've folded that portion into one paragraph: hopefully now the reader will see:

• Care is required when [things move],
• The risk is that developers _observe_ such movements,
• [middle sentence explaining why this is bad]...

[jmorse]

Ping -- AFAIUI we're happy with the meaning in this patch, wording and presentation still needs review?

[jmorse]

Many thanks for the reviews!

docs/SourceLevelDebugging.rst

 
 Here ``!19`` indicates that ``Z`` is declared at line number 5 and column
 number 11 inside of lexical scope ``!18``.  The lexical scope itself resides
 inside of subprogram ``!4`` described above.
 
 The scope information attached with each instruction provides a straightforward
 way to find instructions covered by a scope.
 
+Object lifetime in optimized code
+=================================
+
+In the example above, every variable assignment uniquely corresponds to a
+memory store to the variable's position on the stack. However in heavily
+optimized code LLVM promotes most variables into SSA values, which can
+eventually be placed in physical registers or memory locations. To track SSA
+values through compilation, when objects are promoted to SSA values an
+``llvm.dbg.value`` intrinsic is created for each assignment, recording the
+variable's new location. Compared with the ``llvm.dbg.declare`` intrinsic:
+
+* A dbg.value terminates the effect of any preceeding dbg.values for (any
+  overlapping fragments of) the specified variable.
+* The dbg.value's position in the IR defines where in the instruction stream
+  the variable's value changes.
+* Operands can be constants, indicating the variable is assigned a
+  constant value.
+
+Care must be taken to update ``llvm.dbg.value`` intrinsics when optimization
+passes alter or move other instructions -- the developer will observe such
+changes reflected in the value of variables when debugging the program. For any
+execution of the optimized program, the set of variable values presented to the
+developer by the debugger should not show a state that would never have existed
+in the execution of the unoptimized program, given the same input. Doing so
+risks misleading the developer by reporting a state that does not exist,
+damaging their understanding of the optimized program and undermining their
+trust in the debugger.
+
+Sometimes perfectly preserving variable locations is not possible, often when a
+redundant calculation is optimized out. In such cases, a ``llvm.dbg.value``
+with operand ``undef`` should be used, to terminate earlier variable locations
+and let the debugger present ``optimized out`` to the developer. Withholding
+these potentially stale variable values from the developer diminishes the
+amount of available debug information, but increases the reliability of the
+remaining information.
+ 
+To illustrate some potential issues, consider the following example:
+
+.. code-block:: llvm
+
+  define i32 @foo(i32 %bar, i1 %cond) {
+  entry:
+    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 @llvm.dbg.value(metadata i32 %tval, metadata !1, metadata !2)
+    %g1 = call i32 @gazonk()
+    br label %exit
+  falsebr:
+    %fval = add i32 %bar, 2
+    call @llvm.dbg.value(metadata i32 %fval, metadata !1, metadata !2)
+    %g2 = call i32 @gazonk()
+    br label %exit
+  exit:
+    %merge = phi [ %tval, %truebr ], [ %fval, %falsebr ]
+    %g = phi [ %g1, %truebr ], [ %g2, %falsebr ]
+    call @llvm.dbg.value(metadata i32 %merge, metadata !1, metadata !2)
+    call @llvm.dbg.value(metadata i32 %g, metadata !3, metadata !2)
+    %plusten = add i32 %merge, 10
+    %toret = add i32 %plusten, %g
+    call @llvm.dbg.value(metadata i32 %toret, metadata !1, metadata !2)
+    ret i32 %toret
+  }
+
+Containing two source-level variables in ``!1`` and ``!3``. The function could,
+perhaps, be optimized into the following code:
+
+.. code-block:: llvm
+
+  define i32 @foo(i32 %bar, i1 %cond) {
+  entry:
+    %g = call i32 @gazonk()
+    %addoper = select i1 %cond, i32 11, i32 12
+    %plusten = add i32 %bar, %addoper
+    %toret = add i32 %plusten, %g
+    ret i32 %toret
+  }
+
+What ``llvm.dbg.value`` intrinsics should be placed to represent the original variable
+locations in this code? Unfortunately the the second, third and fourth
+dbg.values for ``!1`` in the source function have had their operands
+(%tval, %fval, %merge) optimized out. Assuming we cannot recover them, we
+might consider this placement of dbg.values:
+
+.. code-block:: llvm
+
+  define i32 @foo(i32 %bar, i1 %cond) {
+  entry:
+    call @llvm.dbg.value(metadata i32 0, metadata !1, metadata !2)
+    %g = call i32 @gazonk()
+    call @llvm.dbg.value(metadata i32 %g, metadata !3, metadata !2)
+    %addoper = select i1 %cond, i32 11, i32 12
+    %plusten = add i32 %bar, %addoper
+    %toret = add i32 %plusten, %g
+    call @llvm.dbg.value(metadata i32 %toret, metadata !1, metadata !2)
+    ret i32 %toret
+  }
+
+However, this will cause ``!3`` to have the return value of ``@gazonk()`` at
+the same time as ``!1`` has the constant value zero -- a pair of assignments
+that never occurred in the unoptimized program. To avoid this, we must terminate
+the range that ``!1`` has the constant value assignment by inserting an undef
+dbg.value before the dbg.value for ``!3``:
+
+.. code-block:: llvm
+
+  define i32 @foo(i32 %bar, i1 %cond) {
+  entry:
+    call @llvm.dbg.value(metadata i32 0, metadata !1, metadata !2)
+    %g = call i32 @gazonk()
+    call @llvm.dbg.value(metadata i32 undef, metadata !1, metadata !2)
+    call @llvm.dbg.value(metadata i32 %g, metadata !3, metadata !2)
+    %addoper = select i1 %cond, i32 11, i32 12
+    %plusten = add i32 %bar, %addoper
+    %toret = add i32 %plusten, %g
+    call @llvm.dbg.value(metadata i32 %toret, metadata !1, metadata !2)
+    ret i32 %toret
+  }
+
+In general, if any dbg.value has its operand optimized out and cannot be
+recovered, then an undef dbg.value is necessary to terminate earlier variable
+locations. Additional undef dbg.values may be necessary when the debugger can
+observe re-ordering of assignments.
+
 .. _ccxx_frontend:
 
 C/C++ front-end specific debug information
 ==========================================
 
 The C and C++ front-ends represent information about the program in a format
 that is effectively identical to `DWARF 3.0
 <http://www.eagercon.com/dwarf/dwarf3std.htm>`_ in terms of information
 content.  This allows code generators to trivially support native debuggers by
 generating standard dwarf information, and contains enough information for
 non-dwarf targets to translate it as needed.
 
 This section describes the forms used to represent C and C++ programs.  Other
 languages could pattern themselves after this (which itself is tuned to
 representing programs in the same way that DWARF 3 does), or they could choose
 to provide completely different forms if they don't fit into the DWARF model.
 As support for debugging information gets added to the various LLVM
 source-language front-ends, the information used should be documented here.
 
 The following sections provide examples of a few C/C++ constructs and the debug
 information that would best describe those constructs.  The canonical
 references are the ``DIDescriptor`` classes defined in
 ``include/llvm/IR/DebugInfo.h`` and the implementations of the helper functions
 in ``lib/IR/DIBuilder.cpp``.
 
 C/C++ source file information
 -----------------------------
 
 ``llvm::Instruction`` provides easy access to metadata attached with an
 instruction.  One can extract line number information encoded in LLVM IR using
 ``Instruction::getDebugLoc()`` and ``DILocation::getLine()``.
 
 .. code-block:: c++
 
   if (DILocation *Loc = I->getDebugLoc()) { // Here I is an LLVM instruction
     unsigned Line = Loc->getLine();
     StringRef File = Loc->getFilename();
     StringRef Dir = Loc->getDirectory();
     bool ImplicitCode = Loc->isImplicitCode();
   }
 
 When the flag ImplicitCode is true then it means that the Instruction has been
 added by the front-end but doesn't correspond to source code written by the user. For example
 
 .. code-block:: c++

 .. code-block:: c
 
   _Alignas(8) int MyGlobal = 100;
 
 a C/C++ front-end would generate the following descriptors:
 
 .. code-block:: text
 
   ;;
   ;; Define the global itself.
   ;;
   @MyGlobal = global i32 100, align 8, !dbg !0
 
   ;;
   ;; List of debug info of globals
   ;;
   !llvm.dbg.cu = !{!1}
 
   ;; Some unrelated metadata.
   !llvm.module.flags = !{!6, !7}
   !llvm.ident = !{!8}
 
   ;; Define the global variable itself
   !0 = distinct !DIGlobalVariable(name: "MyGlobal", scope: !1, file: !2, line: 1, type: !5, isLocal: false, isDefinition: true, align: 64)
 
   ;; Define the compile unit.
   !1 = distinct !DICompileUnit(language: DW_LANG_C99, file: !2,
                                producer: "clang version 4.0.0",
                                isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug,
                                enums: !3, globals: !4)
 
   ;;
   ;; Define the file
   ;;
   !2 = !DIFile(filename: "/dev/stdin",
                directory: "/Users/dexonsmith/data/llvm/debug-info")
 
   ;; An empty array.
   !3 = !{}
 
   ;; The Array of Global Variables
   !4 = !{!0}