[RISCV] Fix wrong type prototype of RVVSlideOneBuiltinSet

[wangpc]

Summary

We need unsigned integer here.

Fixes #64534

[eopXD]

I guess we need to update the test cases too, are you able to generate them by modifying the the generator under riscv-non-isa/rvv-intrinsic-doc?

[wangpc]

In D157476#4572073, @eopXD wrote:

I guess we need to update the test cases too, are you able to generate them by modifying the the generator under riscv-non-isa/rvv-intrinsic-doc?

For this patch, we don't need to update tests because they are using the right integer types and we didn't generate tests that pass immediates to integer arguments.
If we decide to generate/add such kind of tests, then we should post another patch I think.

[eopXD]

I mean

vint8mf8_t test_vslide1up_vx_i8mf8(vint8mf8_t src, int8_t value, size_t vl) {
  return __riscv_vslide1up_vx_i8mf8(src, value, vl);
}

should change into

vint8mf8_t test_vslide1up_vx_i8mf8(vint8mf8_t src, uint8_t value, size_t vl) {
  return __riscv_vslide1up_vx_i8mf8(src, value, vl);
}

[wangpc]

In D157476#4572135, @eopXD wrote:

I mean

vint8mf8_t test_vslide1up_vx_i8mf8(vint8mf8_t src, int8_t value, size_t vl) {
  return __riscv_vslide1up_vx_i8mf8(src, value, vl);
}

should change into

vint8mf8_t test_vslide1up_vx_i8mf8(vint8mf8_t src, uint8_t value, size_t vl) {
  return __riscv_vslide1up_vx_i8mf8(src, value, vl);
}

Why do we need pass unsigned integers when the vector elements are signed? Am I missing something here?

[eopXD]

Sorry, my previous comment was wrong. Please allow me to correct myself here.

---

I went through the v-spec again, it says that:

The vslide1up instruction places the x register argument at location 0 of the destination vector register group, provided that element 0 is active, otherwise the destination element update follows the current mask agnostic/undisturbed policy. If XLEN < SEW, the value is sign-extended to SEW bits. If XLEN > SEW, the least-significant bits are copied over and the high SEW-XLEN bits are ignored.

So I think the righteous fix here is to have the rs1 parameter as type of long, which is to use l in the TableGen.

---

I still think that modifying the generated test cases is needed. If you find it tedious, may I open up a revision that resolves the problem while adding you as co-author since you gave this problem the first try?

[wangpc]

I still think that modifying the generated test cases is needed. If you find it tedious, may I open up a revision that resolves the problem while adding you as co-author since you gave this problem the first try?

@eopXD Yeah, it's OK! I will hand over the work to you, thanks! :-)

[eopXD]

You do agree with my latest comment that long is the righteous fix, right? :)

[wangpc]

In D157476#4575656, @eopXD wrote:

You do agree with my latest comment that long is the righteous fix, right? :)

Oh sorry, somehow I missed it.
I still think that using the element type is the right way. Because user will know the range of effective inserted value.
For example:

vint16m1_t __riscv_vslide1up_vx_i16m1 (vint16m1_t src, int16_t value, size_t vl);
vint16m1_t __riscv_vslide1up_vx_i16m1 (vint16m1_t src, long value, size_t vl);

We can know the range is [-32768, 32767] for the first intrinsic. When the user pass an argument larger than that(e.g. __riscv_vslide1up_vx_i16m1(src, 0xFFFFFFFF, vl)), the compiler can warn the user and may avoid unexpected behaviors.
For the second intrinsic, we can pass a long value. But for SEW<XLEN, the inserted value will be truncated. For example, __riscv_vslide1up_vx_i16m1(src, 0xFFFFFFFF, vl) won't report any warning, but the inserted value is just 0xFFFF, which may be unexpected/confused.
That's just my thought. Issue or proposal can be raised in Github repo if we can't come up with an agreement here. :-)

[eopXD]

I have been misunderstanding the problem the whole time. I see the clear solution you have right now. No changes in the spec / test cases are needed.

clang/include/clang/Basic/riscv_vector.td

: RVVOutOp1BuiltinSet<NAME, "l",
                            ["vx", "v", "vve"]]>,
       RVVOutOp1BuiltinSet<NAME, "l",
                           [["vv", "Uv", "UvUvUv"],
                            ["vx", "Uv", "UvUvUe"]]>;
 
 multiclass RVVSlideOneBuiltinSet
     : RVVOutOp1BuiltinSet<NAME, "csil",
                           [["vx", "v", "vve"],
-                           ["vx", "Uv", "UvUve"]]>;
+                           ["vx", "Uv", "UvUvUe"]]>;
 
 multiclass RVVSignedShiftBuiltinSet
     : RVVOutOp1BuiltinSet<NAME, "csil",
                           [["vv", "v", "vvUv"],
                            ["vx", "v", "vvz"]]>;
 
 multiclass RVVSignedShiftBuiltinSetRoundingMode
     : RVVOutOp1BuiltinSet<NAME, "csil",