[PATCH] D61273: [globalisel] Update the legalizer documentation

[Daniel Sanders via Phabricator via llvm-commits]

dsanders marked 10 inline comments as done.
dsanders added inline comments.


================
Comment at: docs/GlobalISel.rst:343-345
+An API is broadly similar to SelectionDAG/TargetLowering is available but is not
+recommended as a more powerful API is available. The recommended API looks like
+this::
----------------
paquette wrote:
> I think it would be better to mention the SelectionDAG/TargetLowering API after detailing the recommended method. Probably don't want people to think about the unrecommended API before thinking about the recommended API. ;)
I've moved this to a footnote. I don't like the way footnotes render (they look like citations) but at least it's out of the way.


================
Comment at: docs/GlobalISel.rst:358
+
+At the core of this ruleset is the ``LegalityQuery`` which describes the
+instruction. We use a description rather than instruction to both allow other
----------------
paquette wrote:
> If `getActionDefinitionsBuilder` can have its own subsection, then I think this should too for symmetry's sake.
I haven't put this one into a subsection as it would leave the initial section rather short and it felt like it was part of the same high-level overview. Let me know if the latest version still looks like it needs a section here


================
Comment at: docs/GlobalISel.rst:371
+
+The ``getActionDefinitionsBuilder`` function generates a ruleset for the given
+opcode(s) that rules can be added to. If multiple opcodes are given, they are
----------------
paquette wrote:
> I think this is long enough that it deserves its own section? (Is it possible to have another subsection here? If not, meh, whatever.)
There's exactly one left :-)

Having added the various sections the others felt more like a reference and this one felt more like an explanation so I've expanded on this section with examples and advice. Could you take a look at the new text in the 'Rule Processing and Declaring Rules' section?


Repository:
  rL LLVM

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

https://reviews.llvm.org/D61273