[PATCH] D132220: [Assignment Tracking][1/*] Add initial docs for Assignment Tracking

[J. Ryan Stinnett via Phabricator via llvm-commits]

jryans added a comment.

Overall, this looks great! Thanks for writing it up. 😄

Apologies for all of my style nits that follow... 😅



================
Comment at: llvm/docs/AssignmentTracking.md:1
+# Assignment Tracking
+
----------------
Please also link to this new page from some list of pages to make it easier to find, such as `UserGuides.rst` (most likely both in the page list and in the TOC tree). Search for `InstrRefDebugInfo` for a recent example of doing this for another Markdown-formatted doc.


================
Comment at: llvm/docs/AssignmentTracking.md:4
+Assignment Tracking is an alternative technique for tracking variable locations
+through optimisations in llvm. It provides accurate variable locations for
+assignments where a local variable (or a field of one) is the LHS. Indirect
----------------
Nit: When mentioning LLVM (the system, as opposed to a class or namespace) in text, most docs are fairly consistent in spelling it as "LLVM", so it would be good to follow that style here as well. (Apologies if this seems ultra-nitty...! 😅)


================
Comment at: llvm/docs/AssignmentTracking.md:17
+A secondary goal of assignment tracking is to cause minimal additional work for
+llvm pass writers, and minimal disruption to llvm in general.
+
----------------
Does the [guide for pass authors](https://llvm.org/docs/HowToUpdateDebugInfo.html) need any additions or tweaks to cover this new intrinsic? Since this is currently an experimental feature, perhaps it doesn't make sense merge all your pass-author guidance on Assignment Tracking into that existing doc, but at the very least, a few well-placed links to this doc seem like a good idea to me.


================
Comment at: llvm/docs/AssignmentTracking.md:24
+
+**Enable in clang**: -Xclang -fexperimental-assignment-tracking
+
----------------
Nit: As with "LLVM", use the name "Clang" in text.


================
Comment at: llvm/docs/AssignmentTracking.md:26
+
+**Enable in llvm tools**: -experimental-assignment-tracking
+
----------------



================
Comment at: llvm/docs/AssignmentTracking.md:30
+
+### Assignment markers: llvm.dbg.assign
+
----------------



================
Comment at: llvm/docs/AssignmentTracking.md:73
+
+### Instruction link: DIAssignID
+
----------------



================
Comment at: llvm/docs/AssignmentTracking.md:79
+
+`llvm.dbg.assign` intrinsics `use` a `DIAssignID` metadata instance as an
+operand. This way it "refers to" any store-like instruction that has the same
----------------
"use" is not IR syntax, so perhaps normal quotes are better to parallel "refers to" on the next line.


================
Comment at: llvm/docs/AssignmentTracking.md:173
+**promoting** allocas and store/loads: `llvm.dbg.assign` intrinsics implicitly
+describe joined values in memory locations at CFG joins but this is not
+necessarily the case after promoting (or partially promoting) the
----------------



================
Comment at: llvm/docs/AssignmentTracking.md:177
+`llvm.dbg.assign` intrinsics after the resultant PHIs generated during
+promotion. mem2reg already has to do this (with `llvm.dbg.value`) for
+`llvm.dbg.declare`s. Where a store has no linked intrinsic, the store is
----------------



================
Comment at: llvm/docs/AssignmentTracking.md:182
+
+### Lowering llvm.dbg.assign to MIR
+
----------------



================
Comment at: llvm/docs/SourceLevelDebugging.rst:265
+
+The first three arguments are the same as for an `llvm.dbg.value`. The fourth
+argument is a DIAssignID used to reference a store. The fifth is the
----------------
AFAIK, the `*.rst` format needs two ticks for this... (Isn't writing in two document formats fun...?! 🤪)


================
Comment at: llvm/docs/SourceLevelDebugging.rst:266
+The first three arguments are the same as for an `llvm.dbg.value`. The fourth
+argument is a DIAssignID used to reference a store. The fifth is the
+destination of the store (wrapped as metadata), and the sixth is a `complex
----------------
Use ticks around "DIAssignID" here:

```
``DIAssignID``
```


================
Comment at: llvm/docs/SourceLevelDebugging.rst:270
+
+See llvm/docs/InstrRefDebugInfo.md for more info.
+
----------------
You should be able to use the following to link the new page (even though it's in Markdown):

```
See :doc:`AssignmentTracking` for more info.
```

Also, you are currently pointing to the `InstrRefDebugInfo` page, but probably you meant your new `AssignmentTracking` instead...?

(Phabricator wouldn't let me suggest an edit here for some reason... 🤔)


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

https://reviews.llvm.org/D132220