[PATCH] D122291: [VP] Add more cast VPintrinsic and docs.

[Fraser Cormack via Phabricator via llvm-commits]

frasercrmck accepted this revision.
frasercrmck added a comment.
This revision is now accepted and ready to land.

LGTM, thanks!



================
Comment at: llvm/docs/LangRef.rst:20259
+The return type is the type to cast the value to. Both types must be vector of
+:ref:`integer <t_integer>` type. The bit size of the value must be smaller than
+the bit size of the return type. The second operand is the vector mask. The
----------------
ym1813382441 wrote:
> frasercrmck wrote:
> > in `trunc` we explicitly say it cannot be a no-op cast. Should we say that here too?
> vp.trunc must not be a no-op cast, but this is vp.zext, its behavior should be consistent with the basic zext instruction. The basic zext instruction document does not indicate whether it is no-op cast. My understanding is that different targets have different operations.
Ah right I see I misunderstood "no-op cast" then - I thought it was another way of saying the type sizes must be equivalent. Thanks!


================
Comment at: llvm/docs/LangRef.rst:20282
+      %t = zext <4 x i16> %a to <4 x i32>
+      %also.r = select <4 x i1> %mask, <4 x i16> %t, <4 x i16> undef
+
----------------
ym1813382441 wrote:
> frasercrmck wrote:
> > these select ops should be `<4 x i32>`
> sorry, this is my carelessness.
easily done :)


================
Comment at: llvm/test/Verifier/vp-intrinsics.ll:60
 
-define void @test_vp_int_fp_conversions(<8 x i32> %i0, <8 x float> %f0, <8 x i1> %mask, i32 %evl) {
-  %r0 = call <8 x float> @llvm.vp.sitofp.v8f32.v8i32(<8 x i32> %i0, <8 x i1> %mask, i32 %evl)
+define void @test_vp_int_fp_conversions(<8 x i32*> %p0, <8 x i32> %i0, <8 x i64> %i1, <8 x float> %f0, <8 x double> %f1, <8 x i1> %mask, i32 %evl) {
+  %r0 = call <8 x i32> @llvm.vp.fptoui.v8i32.v8f32(<8 x float> %f0, <8 x i1> %mask, i32 %evl)
----------------
ym1813382441 wrote:
> frasercrmck wrote:
> > Feels to me like the name `test_vp_int_fp_conversions` implies it's testing //only// int<->fp conversions. Maybe we should have `test_vp_int_conversions` for `sext`/`zext`/`trunc`/`ptrtoint`/`inttoptr` and `test_vp_fp_conversions` for `fpext`/`fptrunc`?
> I want to use `test_vp_conversions` to include all, is OK?
Fine by me, I don't think it's very important in this test.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122291