[PATCH] D62423: [globalisel][legalizer] Attempt to write down the minimal legalization rules

[Jessica Paquette via Phabricator via llvm-commits]

paquette added inline comments.


================
Comment at: llvm/docs/GlobalISel.rst:405
+GlobalISel's legalizer has a great deal of flexibility in how a given target
+shapes the gMIR that the rest of the backend must handle. However, there are
+a small number of requirements that all targets must meet.
----------------
"GMIR" is used elsewhere in the document, probably best to use that instead of "gMIR"


================
Comment at: llvm/docs/GlobalISel.rst:408-413
+Before discussing the minimum requirements, we'll need some terminology.
+Each target has a set of types which is the union of all possible types produced
+by at least one legal instruction. We'll call this the Producer Type Set.
+Similarly, each target has a set of types which is the union of all possible
+types consumed by at least one legal instruction. We'll call this the Consumer
+Type Set. Both sets are often identical but there's no guarantee of that. For
----------------
Is it possible to use the fancy definition list thing here, or use some formatting to separate Producer Type Set/Consumer Type Set from the rest of the paragraph? (e.g. bold or italics)

It's not terribly, long, but if I was skimming for the definitions, I think it would help.


================
Comment at: llvm/docs/GlobalISel.rst:420
+
+There are three operations produced by the IRTranslator that can increase the
+size of a scalar:
----------------
four


================
Comment at: llvm/docs/GlobalISel.rst:426
+* G_FPEXT
+of these, the only one with mandatory legality is G_ANYEXT. It's required that
+G_ANYEXT is legal for all inputs from the producer type set and all larger
----------------
Since the point of the section is to get the minimum rule set across I think this flow makes a bit more sense:

1. G_ANYEXT and G_TRUNC must have legalization rules
2. Introduce the other instructions
3. Explanations for why you don't have mandatory rules for the other instructions

I think it would be good to have the mandatory instructions in a list rather than a paragraph too, for the sake of skimmability.


================
Comment at: llvm/docs/GlobalISel.rst:450
+
+Within the vector types, there aren't any defined conversions in LLVM-IR as
+vectors are often converted by reinterpreting the bits or by decomposing the
----------------
"LLVM IR" is more idiomatic, I think


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62423