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

Standard Fixed-length Vector Calling Convention Variant #418

Open
wants to merge 11 commits into
base: master
Choose a base branch
from

Conversation

kito-cheng
Copy link
Collaborator

This proposal outlines a new variant of the calling convention specifically designed for fixed-length vectors. The primary aim of this variant is to facilitate the passing of fixed-length vectors through vector registers. This approach is derived from the standard vector calling convention, it uses the same register conventions and argument passing and return value rules.

A key aspect of this variant is the introduction of ABI_VLEN, which denotes the width of a vector register within this convention. The ABI_VLEN is constrained to be no wider than the ISA's VLEN (Vector Length), ensuring compatibility while allowing for flexibility in different implementations. This parameter can be configured via compiler command line options or through function attributes in source code.

The document recommends setting the default ABI_VLEN to 128 bits, acknowledging it as a common minimal requirement while allowing the flexibility for lower VLEN (32 or 64 bits) as permitted by the ISA. This flexibility is crucial for optimizing the utilization of longer VLENs in various cores.

The proposal specifies how fixed-length vector arguments are passed based on their size relative to ABI_VLEN. Vectors smaller than ABI_VLEN are passed in a single vector argument register, while larger vectors are passed in multiple registers, following the LMUL (Length Multiplier) pattern of 2, 4, or 8, depending on their size.

Additionally, the proposal addresses the handling of structs and unions containing fixed-length vectors. Structs with members that are all fixed-length vectors follow the vector tuple type rules if they conform to size constraints. In contrast, unions with fixed-length vectors adhere to the integer calling convention.

@kito-cheng
Copy link
Collaborator Author

@kito-cheng kito-cheng requested a review from lhtin January 4, 2024 09:57
riscv-cc.adoc Show resolved Hide resolved
riscv-cc.adoc Outdated Show resolved Hide resolved
riscv-cc.adoc Show resolved Hide resolved
@@ -329,6 +329,81 @@ type would be passed.
Floating-point registers fs0-fs11 shall be preserved across procedure calls,
provided they hold values no more than ABI_FLEN bits wide.

=== Standard Fixed-length Vector Calling Convention Variant
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The variant itself seems fine, modulo nits, but how are we planning to enable it?

If it's automatically used by -march=rva23 -mabi=ilp32d that will create major compatibility issues for binary distributions that use a fixed ABI and allow mixing packages at different architecture levels (either as an explicit user action, or as an implementation detail when rebuilding the distribution to change the architecture requirement).

If a new -mabi= value is required to enable use of the variant, it will be usable on closed systems where all packages are built at once, but not on binary distributions, since there is no expectation that binary code built with different -mabi= options is interoperable at all. This will include Debian and Alpine and might include Android and Fedora if their ABIs are finalized prior to the acceptance of this PR.

If it's enabled on a per-function basis using an attribute, or automatically for functions not visible across DSO boundaries, then it's effectively part of the definition of the attribute or a compiler implementation detail and may belong in riscv-c-api-doc or gccint, not here.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My expectation is that should be enabled by per-function basis by attribute, and I think that should have a riscv-c-api-doc PR for that, will send that in the next few days.

riscv-cc.adoc Outdated Show resolved Hide resolved
riscv-cc.adoc Outdated Show resolved Hide resolved
@kito-cheng
Copy link
Collaborator Author

ChangeLog:

  • Reorder rule.
  • Pass struct as tuple-type in register only when vector arg reg is
    enough, otherwise passed in reference.
  • Add NOTE for describe what if ABI_VLEN is smaller than VLEN, also
    come with an example.
  • Add NOTE for describe different functions may use different
    ABI_VLEN values.

@kito-cheng
Copy link
Collaborator Author

ChangeLog:

  • Add rule for single fixed-length vector or fixed-length vector array with size 1.
  • Add rule for zero-length fixed-length arrays.
  • Add explicitly rule for fixed-length vector struct as vector tuple type: pass by ref if no enough arg register.

kito-cheng added a commit to kito-cheng/riscv-c-api-doc that referenced this pull request Feb 2, 2024
…n variant

Fixed-length vector are passed via general purposed register or memory
within current ABI design, we proposed a standard fixed-length vector calling
convention variant for passing the fixed-length vector via vector register.

This is the syntax part in the proposal, further detail for that calling
convention variant see riscv-non-isa/riscv-elf-psabi-doc#418
@kito-cheng
Copy link
Collaborator Author

Proposal for function attribute syntax: riscv-non-isa/riscv-c-api-doc#68

riscv-cc.adoc Outdated Show resolved Hide resolved
riscv-cc.adoc Outdated Show resolved Hide resolved
riscv-cc.adoc Outdated Show resolved Hide resolved
riscv-cc.adoc Outdated Show resolved Hide resolved
riscv-cc.adoc Outdated Show resolved Hide resolved

A struct containing just one fixed-length vector array is passed as though it
were a vector tuple type if the size of the base element for the array is less
or equal to 8×ABI_VLEN bit, and the size of the array is less than 8×ABI_VLEN
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
or equal to 8×ABI_VLEN bit, and the size of the array is less than 8×ABI_VLEN
or equal to 4×ABI_VLEN bit, and the size of the array is less than or equal to 8×ABI_VLEN

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since the array would have >=2 elements here, so I think the length can't be 8xABI_VLEN.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Array with length 1 is legal :P

riscv-cc.adoc Outdated
cores with longer VLEN.

A fixed-length vector argument is passed in a vector argument register if the
size of the vector is less than ABI_VLEN bit.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be "no more than ABI_VLEN bits"?

kito-cheng and others added 7 commits September 5, 2024 15:46
This proposal outlines a new variant of the calling convention specifically
designed for fixed-length vectors. The primary aim of this variant is to
facilitate the passing of fixed-length vectors through vector registers. This
approach is derived from the standard vector calling convention, it uses the
same register conventions and argument passing and return value rules.

A key aspect of this variant is the introduction of ABI_VLEN, which denotes the
width of a vector register within this convention. The ABI_VLEN is constrained
to be no wider than the ISA's VLEN (Vector Length), ensuring compatibility while
allowing for flexibility in different implementations. This parameter can be
configured via compiler command line options or through function attributes in
source code.

The document recommends setting the default ABI_VLEN to 128 bits, acknowledging
it as a common minimal requirement while allowing the flexibility for
lower VLEN (32 or 64 bits) as permitted by the ISA. This flexibility
is crucial for optimizing the utilization of longer VLENs in various cores.

The proposal specifies how fixed-length vector arguments are passed based on
their size relative to ABI_VLEN. Vectors smaller than ABI_VLEN are passed in
a single vector argument register, while larger vectors are passed in multiple
registers, following the LMUL (Length Multiplier) pattern of 2, 4, or 8,
depending on their size.

Additionally, the proposal addresses the handling of structs and unions
containing fixed-length vectors. Structs with members that are all fixed-length
vectors follow the vector tuple type rules if they conform to size constraints.
In contrast, unions with fixed-length vectors adhere to the integer calling
convention.
- Reorder rule.
- Pass struct as tuple-type in register only when vector arg reg is
  enough, otherwise passed in reference.
- Add NOTE for describe what if ABI_VLEN is smaller than VLEN, also
  come with an example.
- Add NOTE for describe different functions may use different
  ABI_VLEN values.
- Add rule for single fixed-length vector or fixed-length vector array
  with size 1.
- Add rule for zero-length fixed-length arrays.
- Add explicitly rule for fixed-length vector struct as vector tuple type:
  pass by ref if no enough arg register.
Co-authored-by: Brandon Wu <songwu0813@gmail.com>
Signed-off-by: Kito Cheng <kito.cheng@gmail.com>
@@ -467,7 +467,7 @@ with an optimized library with larger ABI_VLEN for better utilization of those
cores with longer VLEN.

A fixed-length vector argument is passed in a vector argument register if the
size of the vector is less than ABI_VLEN bit.
size of the vector is no more than ABI_VLEN bits.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just want to double check, I wasn't concerned about the wording but it seemed to me like it should be vec_size <= ABI_VLEN and not vec_size < ABI_VLEN. Unless that was intentional?

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

Successfully merging this pull request may close these issues.

6 participants