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

[Eli Friedman via Phabricator via llvm-commits]

efriedma added inline comments.


================
Comment at: llvm/docs/LangRef.rst:14664
+
+      <W x T> llvm.vp.<opcode>.*(<W x T> %x, <W x T> %y, <W x i1> %mask, i32 %evl)
+
----------------
simoll wrote:
> sdesmalen wrote:
> > jdoerfert wrote:
> > > sdesmalen wrote:
> > > > I expect this to also work for scalable vectors, so maybe add a case here for `<vscale x W x T>` as well?
> > > If that support is added later, I would add the wording later too.
> > @simoll is anything special needed to support scalable vectors in this context?
> > @simoll is anything special needed to support scalable vectors in this context?
> 
> The intrinsics support scalable vectors out of the box. I've added VP intrinsics with scalable types to the individual operations below.
I assume for scalable vectors, the range of legal values for `%evl` is the runtime vector length?  Maybe worth noting explicitly.


================
Comment at: llvm/docs/LangRef.rst:15296
+example if ``%evl > W``.  The explicit vector length (%evl) is only effective
+when it is not the IR constant ``i32 -1``, and, when that is the case, it
+creates a mask, %EVLmask, with all elements ``0 <= i <= %evl`` set to True, and
----------------
I'm not happy with distinguishing between %evl equal to -1, vs. the IR constant -1.  I mean, we can, but there are complications for other passes like, for example, constant hoisting.  We have immarg for arguments that are *always* immediates, but we don't have a way to mark an argument that's *sometimes* an immediate.

I don't have any great suggestions here... but one idea is to add a second type overload to the intrinsics: `i32` if the evl is present, `{}` if the evl is absent. So `@llvm.vp.add.v4i32.i32` is an add with an evl parameter, and `@llvm.vp.add.v4i32.sl_s` is an add without an evl parameter.  I think that expresses the semantics correctly, but maybe it's not so readable.


================
Comment at: llvm/docs/LangRef.rst:15316
+Some targets, such as AVX512, do not support the %evl parameter in hardware.
+The use of an effective %evl is disencouraged for those targets.  The function
+``TargetTransformInfo::hasActiveVectorLength()`` returns true when the target
----------------
"discouraged"


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69891