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

[Simon Moll via Phabricator via llvm-commits]

simoll marked 3 inline comments as done.
simoll added a comment.

Thanks for the review!
I'll update shortly with an explicit mention of scalable vector types in the langref.



================
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
----------------
efriedma wrote:
> 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.
> 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.

We could make `TTI:getIntIMmCostIntrin` return `0` whenever `i32 -1`  is passed in VP intrinsics for evl.

  /// Return the expected cost of materialization for the given integer
  /// immediate of the specified type for a given instruction. The cost can be
  /// zero if the immediate can be folded into the specified instruction.
   int getIntImmCostIntrin(Intrinsic::ID IID, unsigned Idx, const APInt &Imm,
                             Type *Ty) const;


In any case, the special handling of `i32 -1` is really a makeshift solution. Mid to long term, we want to make evl (and the mask) argument optional parameters (https://lists.llvm.org/pipermail/llvm-dev/2020-February/139110.html).

> 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. 
This might solve the representation issue in the IR. However, doing so would break the 1:1 correspondence between instruction opcodes and VP intrinsics, which is a nice property to have when optimizing VP code (eg for the generalized pattern matching logic in the [[ https://reviews.llvm.org/D57504 | VP reference patch ]]).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69891