[PATCH] D31761: [globalisel][tablegen] Revise API for ComplexPattern operands to improve flexibility.

[Daniel Sanders via Phabricator via llvm-commits]

dsanders added a comment.

In https://reviews.llvm.org/D31761#722903, @ab wrote:

> I'm slightly worried that this makes the selector functions a bit harder to read, but this seems like a nice simplification otherwise.
>
> One question: does this prevent us from ever selecting operands in-place? The previous implementation seems amenable to that (by only assigning the complex-pattern MO instead of adding them all to a new MI).  But now we'd be forced to always append operands, right?


I think it's still possible to handle the mutation cases we could potentially support before this patch. Supporting one matched operand being rendered as one operand can be supported with a little additional information to specify which operand to replace* and a tablegen field to indicate that the renderer supports mutation. However, it's quite common for one matched operand to be rendered as multiple operands (e.g. for addressing modes like addr+offset, or for ARM's immediates). I'm not sure this case is efficiently supportable before or after this patch since we'd need to make room for the additional operands and the underlying storage is an array.

*This gets trickier when multiple instructions are emitted by a rule though. We'd need to decide which instruction to mutate.

> This also means that we can remove MO_Placeholder, yes?

That's right. I'll update the patch to do that.

---

I've been using this mechanism more thoroughly on an out-of-tree target over the last few days and there's an issue that I need to find a solution to. At the moment, we can create new instructions in the renderer but there's no way to have the instruction selector process them. For example, we can emit a target instruction to load an immediate but we can't create a legal G_CONSTANT and ask the instruction selector to figure out some code for it. I'm currently thinking the instruction selector should have a work list we can add new instructions to and only when the work list is empty would it move on to the next instruction in the BB.

It would also be nice to be able to have the legalizer run on newly-created instructions so targets don't end up implementing parts of legalization in the instruction selector like we've seen in SelectionDAG.


https://reviews.llvm.org/D31761