[PATCH] D63255: [ARM] Select MVE add and sub

[Simon Tatham via Phabricator via llvm-commits]

simon_tatham added a comment.

In D63255#1548264 <https://reviews.llvm.org/D63255#1548264>, @dmgreen wrote:

> Simon already mentioned that we could put the patterns into the instructions if we can sort out things like default class parameters.


I still haven't managed to form a definite opinion on whether that's better, though!

In favour of putting ISel patterns inside the instruction records they go with:

- readability: all the information about an instruction is in the same place and easy to find
- DRYness: when the instruction applies to half a dozen different types with suffixes like `.f16`, `.s32` and the like, the pattern is automatically replicated across the same set of types, without having to write a separate repetition elsewhere (`foreach` or multiclass or whatever) and make sure to keep the two in sync.

The unusual problem with MVE is the predication. Almost all the vector instructions will ultimately want at least one ISel pattern corresponding to one of the ACLE intrinsics (or rather, to an LLVM intrinsic which the ACLE one will have been converted into by a combination of clang and a header file) which allows the predicated version of the instruction to be used by C code. But some instructions //also// want patterns corresponding to existing DAG node types, such as ordinary `add` on vector types – and you can't put //both// patterns inside the instruction record, because the `Pattern` field only has room for one. (It's a `list<dag>`, but the elements of the list are parts of the same pattern, not independent alternatives.)

So is it better to put the intrinsic patterns in the instructions? Or the patterns involving standard DAG nodes (if any)? Or neither, on the basis that if you can't have all the patterns in the instructions then you should instead keep them near each other? No idea, really.

(Also, there's that problem where operands whose default values are specified by virtue of an `OperandWithDefaultOps` class can't be overridden by ISel patterns. So the intrinsics for the predicated forms of the instructions can't currently be written as ISel patterns at all. But I think that's fixable: I have a draft patch to TableGen already that //should// remove that constraint, and I'll upload it once I find time to test it a bit more thoroughly.)


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

https://reviews.llvm.org/D63255