Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Renaming the VL arguments to AVL #348

Open
lukel97 opened this issue Jul 3, 2024 · 6 comments
Open

Renaming the VL arguments to AVL #348

lukel97 opened this issue Jul 3, 2024 · 6 comments

Comments

@lukel97
Copy link

lukel97 commented Jul 3, 2024

§5.2 clarifies that the size_t vl argument is used to specify the AVL:

The intrinsics programmer specifies an "application vector length (AVL)" using the argument size_t vl.

I can't speak for others but I've often gotten vl (the register) confused with AVL (the vsetvli argument), and it would appear that there is precedent for clarifying this in the note below:

The actual value written to the vl control register is an implementation defined behavior and is typically not known until runtime. The actual setting of vl, given the provided AVL through the parameter, follows the rules 27 in the specification.

Given this, would it be worthwhile renaming the vl arguments to avl so that we call it what it is?

If it's just a matter of renaming an argument I think this should be backwards compatible.

@topperc
Copy link
Collaborator

topperc commented Jul 3, 2024

For most use cases it probably isn't really the AVL because it came from a vsetvli intrinsic that already clipped it. Though I don't have a better name.

@lukel97
Copy link
Author

lukel97 commented Jul 3, 2024

For most use cases it probably isn't really the AVL because it came from a vsetvli intrinsic that already clipped it.

Isn't that still the AVL though? The compiler might fold two equivalent vsetvlis into one, but IIUC that's still the same as setting the AVL to the output VL of a vsetvli.

@topperc
Copy link
Collaborator

topperc commented Jul 3, 2024

I was thinking about it from the perspective of the programmer that probably wrote something like

while (n != 0) {
  size_t vl = vsetvli_e8m1(n);
  n -= vl;

  a = vadd(a, b, vl);
}

So to them it is the vl because there is a separate AVL they passed to the vsetvli intrinsic. Does the argument to the intrinsics cause confusion since its not the AVL from the programmers perspective in most cases.

I don't think I recommend passing an unclipped AVL to the intrinsics even though it is supported. Do our vsetvli optimizations work as well if you do that?

@lukel97
Copy link
Author

lukel97 commented Jul 3, 2024

I don't think I recommend passing an unclipped AVL to the intrinsics even though it is supported. Do our vsetvli optimizations work as well if you do that?

I'm not sure. I agree though that clipping the AVL beforehand is what programmers should continue to do, if we were to rename the argument I was thinking that they would still write the same

while (n != 0) {
  size_t vl = vsetvli_e8m1(n);
  n -= vl;

  a = vadd(a, b, vl);
}

But the intrinsic signature would just make it more clear that they're setting the AVL to the precalculated vl.

Does the argument to the intrinsics cause confusion since its not the AVL from the programmers perspective in most cases.

I think it's more likely to cause confusion if they're not made aware that it's the AVL, e.g. if they accidentally change the SEW/LMUL ratio:

size_t vl = vsetvli_e32m1(n);
__riscv_vadd_vv_i32m1(a, b, vl); // vl guaranteed to equal vl
__riscv_vadd_vv_i64m1(c, d, vl); // vl not guaranteed to equal vl

@rofirrim
Copy link
Collaborator

We typically use vsetvli in most of our rvv applications so for me it makes sense to call that operand vl.

But I see how it can be confusing in some cases. Especially when using fixed vectors and an assumed VLEN, I always find a bit jarring, even if perfectly correct, that I can just use an integer literal as the vl operand, given how used I am to vsetvli.

I guess that vl operand is conceptually more like "a value in the corresponding range [0, vlmax(sew, lmul)] of the intrinsic" (otherwise the value may be clipped and it is not obvious what happens with the operation). That said, I don't have a better name though for me this is closer to vl than to AVL.

@rofirrim
Copy link
Collaborator

Can we revisit this after 1.0?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants