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

[Daniel Sanders via Phabricator via llvm-commits]

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


================
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
----------------
paquette wrote:
> 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.
That makes sense. I'll re-work it to that


================
Comment at: llvm/docs/GlobalISel.rst:456-457
+
+There are no legality requirements for G_BUILD_VECTOR, or G_BUILD_VECTOR_TRUNC
+since these can be lowered to target instructions which are legal by definition.
+The same reasoning also allows G_UNMERGE_VALUES to lack legality requirements
----------------
arsenm wrote:
> I don't follow this. How do you know they can be lowered to target instructions?
I'm trying to convey that even if the target doesn't use `scalarize()` to eliminate the vector, there's multiple choices on how to handle G_BUILD_VECTOR and no requirement to pick a specific method. In particular there's no requirement for the end point of legalization to be a `.legal*()` rule that marks the G_BUILD_VECTOR legal. An alternative is `.custom*()` that replaces it with a sequence of target instructions. The overall effect is that there aren't any legal G_BUILD_VECTOR's but vectors are still supported.

I guess something like this would be clearer:
  There are no legality requirements for G_BUILD_VECTOR, or G_BUILD_VECTOR_TRUNC since these can be scalarized, lowered to other instructions, or lowered to target instructions.


================
Comment at: llvm/docs/GlobalISel.rst:464-465
+
+There are no minimum rules for pointers since G_INTTOPTR and G_PTRTOINT can
+be lowered to COPY (or to nothing using replaceAllUses()) by the legalizer.
+
----------------
arsenm wrote:
> arsenm wrote:
> > This ignores the existence of non-integral pointers
> This should also probably say "selected" to copy, since IIRC COPY is not allowed to convert the type with generic virtual registers
> This ignores the existence of non-integral pointers

How are those represented in gMIR? I haven't had to deal with them

> This should also probably say "selected" to copy, since IIRC COPY is not allowed to convert the type with generic virtual registers

That's a good point, it needs to be constrained to a register class otherwise the types have to match.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62423