[PATCH] D102158: [DebugInfo][Docs] Add corrected docs for how InstrRefBasedLDV works

[Adrian Prantl via Phabricator via llvm-commits]

aprantl added a comment.

I only had time for a few superficial comments so far and this deserves an in-depth read, but I greatly appreciate the time and effert that went into putting this to paper!



================
Comment at: llvm/docs/BadlyBehaved.tikz:1
+\begin{tikzpicture}
+	\begin{pgfonlayer}{nodelayer}
----------------
Might be nice to add the command line that produced the png as a comment here.
Maybe more LLVM readers are familiar with Graphviz dote files, since LLVM produces them, but I don't have a strong opinion.


================
Comment at: llvm/docs/InstrRefLiveDebugValues.md:2
+This document is an explanation of what the LiveDebugValues (LDV) pass does,
+both the "old" variable location based one, and the "new" value tracking one.
+The reader is expected to be fairly familiar with LLVM debug-info primitives,
----------------
References to old and new don't usually age well — could we just call them by name?


================
Comment at: llvm/docs/InstrRefLiveDebugValues.md:11
+
+Motivation: While I'm confident that value tracking LDV is an improvement, it's
+also a large amount of C++ that other people in the community will be forced to
----------------
Checked-in documentation should probably not be written from a personal point of view. It might be better to take a neutral stance and sound like it's just describing facts.


================
Comment at: llvm/docs/InstrRefLiveDebugValues.md:19
+at the end, in LiveDebugValues. It's worth noting that gcc took a similar
+approach many years ago [1].
+
----------------
It might be best not to directly link to a differently licensed project.


================
Comment at: llvm/docs/InstrRefLiveDebugValues.md:23
+
+Throughout the process of compilation, LLVM maintains two categories ofu
+information about variable locations:
----------------
of


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102158