[PATCH] D85838: New TableGen Programmer's Reference document

[Chris Lattner via Phabricator via llvm-commits]

lattner accepted this revision.
lattner added a comment.
This revision is now accepted and ready to land.

This is really fantastic work, thank you for doing this!



================
Comment at: llvm/docs/TableGen/ProgRef.rst:13
+
+This document describes the LLVM TableGen facility in complete detail. It is
+intended for the programmer who is using TableGen to produce tables for a
----------------
"complete detail" might be a stretch :-). People will always want more, and this will replace the existing documentation - you don't need to reference the old thing.

The first paragraph of the introduction should be an introduction to tablegen, something closer to your second paragraph.


================
Comment at: llvm/docs/TableGen/ProgRef.rst:62-63
+the output of those backends.
+
+All the fields in a concrete record usually have values. A backend processes
+some subset of the concrete records built by the TableGen parser and emits
----------------
I'd phrase it slightly differently maybe something like:

It is typical for abstract records to have many missing fields which are then filled in when they are instantiated into a concrete record, but even a concrete record may have fields with the value of `?`.


================
Comment at: llvm/docs/TableGen/ProgRef.rst:65
+some subset of the concrete records built by the TableGen parser and emits
+the output files. In a complex program such as LLVM, there can be many
+concrete records and some of them can have an unexpectedly large number of
----------------
I'd replace "In a complex program such as LLVM," with "In a complex use case like the LLVM code generator,"


================
Comment at: llvm/docs/TableGen/ProgRef.rst:92
+
+`Appendix B: Sample Record`_ illustrates a complex record in the Intel x86 target
+and the simple way in which it is defined.
----------------



================
Comment at: llvm/docs/TableGen/ProgRef.rst:100
+statements, comments, and blank lines (see `Lexical Analysis`_). The standard file
+extension for TableGen files is ``.td``.
+
----------------
I'd mention the preprocessor in this section


================
Comment at: llvm/docs/TableGen/ProgRef.rst:215
+   IncludeDirective: "include" `TokString`
+
+
----------------
I'd mention #ifndef/define as well since this is an overview


================
Comment at: llvm/docs/TableGen/ProgRef.rst:309
+  two consecutive tokens, with values ``1`` and ``-5``,
+  instead of "1", "-", and "5".
+
----------------
It would be nice to deprecate and remove this at some point, `..` is much nicer :-)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85838