[PATCH] D57504: RFC: Prototype & Roadmap for vector predication in LLVM
Simon Moll via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Mon Nov 25 02:43:02 PST 2019
simoll added a comment.
Moving the discussion from the integer patch alley to the main RFC as this is about the general design of VP intrinsics.. it's about having a passthru operand (as in `llvm.masked.load`) and whether `%evl` should be a parameter of the intrinsics or modelled differently.
> @SjoerdMeijer https://reviews.llvm.org/D69891#inline-636845
> and if I'm not mistaken we are now discussing if undef here should be undef or a passthru
> @rkruppe https://reviews.llvm.org/D69891#inline-637215
> I previously felt that passthru would be nice to have for backend maintainers (including myself) but perhaps not worth the duplication of IR functionality (having two ways to do selects). However, given the differences I just described, I don't think "just use select" is workable.
Ok. I do agree that having passthru simplifies isel for certain architectures (and legal combinations of passthru value, type, and operations..) but:
VP intrinsics aren't target intrinsics: they are not supposed to be a way to directly program any specific ISA in the way of a macroassembler, like you would do with `llvm.x86.*` or `llvm.arm.mve.*` or any other. Rather, think of them as regular IR instructions. Pretend that anything we propose in VP intrinsics will end up as a feature of a first-class LLVM instructions. Based on that i figured that one VP intrinsics should match one IR instructions plus predication, nothing more.
- If we had predicated IR instructions, would we want them to have a passthru operand?
- The prototype shows that defining VP intrinsics with undef-on-masked-out makes it straightforward to generalize InstSimplify/InstCombine/DAGCombiner such that they can optimize VP intrinsics. If you add a passthru operand then logically VP intrinsics start to behave like two instructions: that could be made work but it would be messier as you'd have to peek through selects, etc.
> @sdesmalen https://reviews.llvm.org/D69891#1750287
> If we want to solve the select issue and also keep the intrinsics simple, my suggestion was to combine the explicit vector length with the mask using an explicit intrinsic like @llvm.vp.enable.lanes. Because this is an explicit intrinsic, the code-generator can simply extract the %evl parameter and pass that directly to the instructions for RVV/SXA. This is what happens for many other intrinsics in LLVM already, like masked.load/masked.gather that support only a single addressing mode, where it is up to the code-generator to pick apart the value into operands that are suited for a more optimal load instruction.
>
> Without having heard your thoughts on this suggestion, I would have to guess that your reservation is the possibility of LLVM hoisting/separating the logic that merges predicate mask and %evl value in some way. That would mean having to do some tricks (think CodeGenPrep) to keep the values together and recognizable for CodeGen. And that's the exact same thing we would like to avoid for supporting merging/zeroing predication, hence the suggestion for the explicit passthru parameter.
That's not quite the same:
`%evl` is mapped to a hardware register on SX-Aurora. We cannot simply reconstitute the `%evl` from any given mask, if `%evl` is obscured it makes **all** operations that depend on it less efficient because we need to default to the full vector length. Now, if the `select` is separated from the VP intrinsic, you simply emit one select instruction (and it should be possible to hoist it back and merge it with the VP intrinsic in most cases (.. and you probably want an optimization that does that in anyway because there will be code with explicit selects even with passthru)). Besides, if the select is folded with an instruction that is subsequently simpler then that's actually an argument in favor of explicit selects: passthru makes this implicit.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D57504/new/
https://reviews.llvm.org/D57504
More information about the llvm-commits
mailing list