[PATCH] D59790: [DebugInfo][Docs] Document how variable location metadata is transformed through target codegen

[Jeremy Morse via Phabricator via llvm-commits]

jmorse added inline comments.


================
Comment at: docs/SourceLevelDebugging.rst:519
 
+Compiling debug information to CodeGen targets
+==============================================
----------------
vsk wrote:
> Wdyt of keeping with the title of this patch and calling this 'How variable location metadata is used' (or, transformed)?
Works for me, I guess that's less abstract,


================
Comment at: docs/SourceLevelDebugging.rst:532
+to a mapping from each instruction to a set of variable locations, which can
+then be encoded in the desired file format. Three transformations account for
+the most serious changes in debugging information, as explained below.
----------------
vsk wrote:
> Isn't it the reverse? I thought we mapped variable locations to instruction ranges. Also, not sure it's necessary to mention encoding/serializing here.
True,


================
Comment at: docs/SourceLevelDebugging.rst:540
+
+Within IR, variable locations or their memory address are always identified by
+a Value. When instruction selection occurs and the IR becomes encoded in a MIR
----------------
vsk wrote:
> Can we drop the 'or', or is there a need to distinguish 'location' from 'memory address'?
Hmm, all I really wanted to get across was that there are multiple categories of machine locations, no real need to distinguish different IR locations.


================
Comment at: docs/SourceLevelDebugging.rst:554
+otherwise transformed into a non-register, the variable location becomes
+undefined.
+
----------------
vsk wrote:
> Can you clarify what is meant by undefined? Is this something that gets patched up later, or is the assignment lost?
Revised patch now describes locations as "unavailable", then adds a sentence defining what that means. I've also explicitly linked dbg.value(undef) and $noreg in the DBG_VALUE description below.


================
Comment at: docs/SourceLevelDebugging.rst:575
+
+The position at which the DBG_VALUEs are inserted should correspond to the
+positions of their matching ``llvm.dbg.value`` intrinsics in the IR block.
----------------
dstenb wrote:
> Is it worth mentioning that there are some exceptions with parameter debug values (`ArgDbgValues`), or is that too detailed?
Given the amount of code that juggles parameters, it's good to mention. I think it's also a design goal as well as an implementation detail.


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

https://reviews.llvm.org/D59790