[PATCH] D69891: [VP,Integer,#1] Vector-predicated integer intrinsics

[Simon Moll via Phabricator via llvm-commits]

simoll added a comment.

In D69891#1852795 <https://reviews.llvm.org/D69891#1852795>, @andrew.w.kaylor wrote:

> I know it's very late for me to be bringing this up, but I have a couple of broad questions -- one minor, one less minor.
>
> First, when we introduced the constrained FP intrinsics Chandler suggested inserting ".experimental" in the names because of the likelihood that something would need to change before we were done and the "experimental" modifier would make the auto-upgrader handling slightly less ugly. That same reasoning seems like it would apply here.


I don't mind changing the name to 'llvm.experimental.vp.add', etc . The mid to long-term goal is first-class VP instructions in any case.

> Second, I'm not 100% comfortable with how the explicit vector length argument is represented. Yes, I know, I should have brought this up about a year ago. I don't have any objections to the existence of this argument. I understand that we need some way to handle it. I'm just wondering if we can handle it in some way that makes it more like an optional argument. I'm not entirely clear where this value comes from, but it seems like whatever generated it must know that we're generating SVE code, right? My concern is that for targets that don't do SVE kinds of things we still have this entirely redundant, if not meaningless, operand to carry around. What would you think of this being provided as an operand bundle when it is needed?

Operand bundles are not the same as optional parameters. Operand bundles pessimistically imply side-effects where as a plain `i32` argument is innocuous to optimizations:

> https://llvm.org/docs/LangRef.html#operand-bundles
> 
> - Calls and invokes with operand bundles have unknown read / write effect on the heap on entry and exit (even if the call target is readnone or readonly), unless they’re overridden with callsite specific attributes.

That is reasonable for constrained fp intrinsics, which always have such a dependence. Many VP intrinsics however do not have any side effects or only inspect/modify memory through their pointer arguments, etc.

In D69891#1854257 <https://reviews.llvm.org/D69891#1854257>, @rkruppe wrote:

> <snip>
>
> - It has no impact on code generation, since the canonical way to indicate that the "EVL feature" is not used by an instruction (the EVL argument being constant -1) is trivial to identify.
>
>   So I think that making this argument optional accomplishes nothing, it only makes support for targets that do care (RISC-V V, SX-Aurora VE) more complicated.


Yep. Thanks for clarifying this. Once LLVM supports optional parameters without adverse effects, we can re-visit this discussion until then i'd strongly prefer the EVL to be an explicit parameter.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69891