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

Add misc macro for vector extensions. #21

Merged
merged 1 commit into from
Feb 11, 2022

Conversation

kito-cheng
Copy link
Collaborator

@kito-cheng kito-cheng commented Aug 19, 2021

Changes:
2021/08/19: Apply Fraser's fix.
2021/08/19: Change prefix to __riscv_v_


Background

RVV 1.0 has add few more extension (zvl*b and zve*) to give the promise about the EEW and VLEN, and this PR intend to expose those info by predefined marco.

https://github.com/riscv/riscv-v-spec/blob/master/v-spec.adoc#sec-vector-extensions


Introduce __riscv_v_min_vlen, __riscv_v_max_eew and __riscv_v_max_eew_fp
macro, for let programer easier to get vector related information, like:

  • What's the minimal VLEN value is guaranteed current architecture
    extension?
  • What's the maximal available EEW is guaranteed current
    architecture extension?
  • What's the maximal available EEW for floating-point operation is
    guaranteed current architecture extension?

It's possible to get the information by testing serveral architecture extension
test macro, but the life can be simpler if toolchain can help, for
example, if you want to check the minimal VLEN value via those macro:

 #if defined(__riscv_zvl512b)
   #define __riscv_v_min_vlen 512
 #elif defined(__riscv_zvl256b)
   #define __riscv_v_min_vlen 256
 #elif defined(__riscv_zvl128b) || defined(__riscv_v)
   #define __riscv_v_min_vlen 128
 #elif defined(__riscv_zvl64b) || defined(__riscv_zve64x) || \
       defined(__riscv_zve64f) || defined(__riscv_zve64d)
   #define __riscv_v_min_vlen 64
 #elif defined(__riscv_zvl32b) || defined(__riscv_zve32x) || \
       defined(__riscv_zve32f)
   #define __riscv_v_min_vlen 32
 #endif

And it's not scalable solution since in theory we can have up to zvl32768b...

@kito-cheng
Copy link
Collaborator Author

@rdolbeau @nick-knight @HanKuanChen @topperc @Hsiangkai @zakk0610 @JerryShih @rjiejie @rofirrim you might interested on this.

@jrtc27 @asb @luismarques you might not so interested, but we need LLVM folks :p

@HanKuanChen
Copy link

Because these macros belong to v extension, should not we add _v to the macros?
e.g., __riscv_v_min_vlen, __riscv_v_max_eew and __riscv_v_max_eew_fp

@rdolbeau
Copy link

Will there be some mechanism so that when compiling for zvl512b, the binary won't be able to run on a zvl256b-or-smaller machine?

@kito-cheng
Copy link
Collaborator Author

Will there be some mechanism so that when compiling for zvl512b, the binary won't be able to run on a zvl256b-or-smaller machine?

We will encode _zvl*b in Tag_RISCV_arch@RISC-V attribute, and every executable and shared library will contain that info, so in theory we could use that to check on linux kernel and glibc, unfortunately both are not implement yet...but I believe we will have that in future.

[1] https://github.com/riscv/riscv-elf-psabi-doc/blob/master/riscv-elf.md#list-of-attributes

@kito-cheng
Copy link
Collaborator Author

Because these macros belong to v extension, should not we add _v to the macros?
e.g., __riscv_v_min_vlen, __riscv_v_max_eew and __riscv_v_max_eew_fp

Sounds good to me, let me update :)

@kito-cheng kito-cheng force-pushed the v-ext-marco branch 2 times, most recently from 549e7e1 to b60c3fd Compare August 19, 2021 10:03
@kito-cheng
Copy link
Collaborator Author

Changes:

  • Change prefix to __riscv_v_

riscv-c-api.md Outdated Show resolved Hide resolved
riscv-c-api.md Outdated Show resolved Hide resolved
@kito-cheng
Copy link
Collaborator Author

Changes:

  • Apply Fraser's suggestion.

riscv-c-api.md Outdated

- `__riscv_v_max_eew_fp` is 64 if `V` or `Zve64d` extension is available.
- `__riscv_v_max_eew_fp` is 32 if `Zve32f` or `Zve64f` extension is available.
- `__riscv_v_max_eew_fp` is 0 if `Zve64x` or `Zve32x` extension is available.

Choose a reason for hiding this comment

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

Why does it define as 0, instead of undefined?

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 intention is make sure this marco is always defined if vector extension is available, so we only need to check __riscv_v_max_eew_fp is larger than our requirement once we ensure the vector extension is available.

e.g.

#ifdef __riscv_v_max_eew
#error "NO vector extension supported"
#endif

#if __riscv_v_max_eew_fp > 64
...
#endif

Rather than

#ifdef __riscv_v_max_eew
#error "NO vector extension supported"
#endif

#if defined(__riscv_v_max_eew_fp) && __riscv_v_max_eew_fp >= 64
...
#endif

@rjiejie
Copy link

rjiejie commented Sep 3, 2021

+1

riscv-c-api.md Outdated Show resolved Hide resolved
@topperc
Copy link
Contributor

topperc commented Oct 26, 2021

Is max_eew the right term here or should it be elen? @aswaterman @kasanovic

@nick-knight
Copy link

nick-knight commented Oct 26, 2021

There are other wrinkles related to ELEN and max_eew. For example, in every (standard) vector extension the maximum EEW for the indices in an indexed load/store is XLEN, which can be less than max_eew. As another example, the Zve64* extensions don't support a handful of instructions with EEW = 64 operands. Neither ELEN nor max_eew is sufficiently fine-grained to convey this information. This makes me question the value of these EEW macros, versus macros that simply indicate which extension is implemented (V, Zve32{x,f}, Zve64{x,f,d}), from which the programmer can deduce which EEWs are supported in which contexts.

riscv-c-api.md Outdated Show resolved Hide resolved
@kito-cheng
Copy link
Collaborator Author

My intention is to simplify the marco feature checking by __riscv_v_max_eew and __riscv_v_max_eew_fp instead of checking bunch of march.

Checking ELEN / EEW support 64 require checking __riscv_v, __riscv_zve64d, __riscv_zve64f and __riscv_zve64x, and the list might even longer when we add extension for z*inx[1] like zve32fx.

There is a list of instruction with EEW=64 are excluded from zve64*, but that still simplify most case, user just need to check !defined(__riscv_v) for those instruction instead of __riscv_zve64d, __riscv_zve64f and __riscv_zve64x.

So I would prefer to keep those marco.

[1] riscv/riscv-v-spec@6fedb86

@nick-knight
Copy link

nick-knight commented Oct 27, 2021

I agree that it might be cumbersome for the user to check multiple of the six extant __riscv_{v,zve*} macros, and I agree that future vector extensions based on "Z*inx" might complicate this further.

I guess my concerns are really just the two special cases I mentioned:

  • eight EEW = 64 instructions ({vmulh*,vsmul}*) are excluded from Zve64*;
  • index-EEW = 64 instructions (v*x*ei64*) are excluded from V and Zve* when XLEN = 32.

Users of these instructions will need to be aware of these details and check other macros besides __riscv_max_eew. I don't have a strong opinion on which API has larger cognitive burden, so I don't object to the current proposal.

@kito-cheng
Copy link
Collaborator Author

Changes:

  • Fix typos.

@kito-cheng
Copy link
Collaborator Author

@aswaterman @kasanovic @nick-knight do you think max_eew/max_eew_fp is good enough or it should be elen/elen_fp?

Copy link

@nick-knight nick-knight left a comment

Choose a reason for hiding this comment

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

@kito-cheng I have some suggestions for the wording and formatting. I don't think I've changed any of the semantics.

I still think we should have __riscv_zve* macros, but that can be a separate PR.

riscv-c-api.md Outdated Show resolved Hide resolved
riscv-c-api.md Outdated Show resolved Hide resolved
riscv-c-api.md Outdated Show resolved Hide resolved
riscv-c-api.md Outdated Show resolved Hide resolved
riscv-c-api.md Outdated Show resolved Hide resolved
riscv-c-api.md Outdated Show resolved Hide resolved
riscv-c-api.md Outdated Show resolved Hide resolved
riscv-c-api.md Outdated Show resolved Hide resolved
riscv-c-api.md Outdated Show resolved Hide resolved
riscv-c-api.md Outdated Show resolved Hide resolved
@kito-cheng
Copy link
Collaborator Author

@nick-knight

@kito-cheng My suggestion got broken somehow, I guess I must have been reviewing an older commit?

I got mail but not saw the message on github, anyway I'll merge those stuffs, thanks for your review comment!

@kito-cheng
Copy link
Collaborator Author

Changes:

eopXD pushed a commit to llvm/llvm-project that referenced this pull request Jan 15, 2022
`zvl` is the new standard vector extension that specifies the minimum vector length of the vector extension.
The `zvl` extension is related to the `zve` extension and other updates that are added in v1.0.

According to riscv-non-isa/riscv-c-api-doc#21,
Clang defines macro `__riscv_v_min_vlen` for `zvl` and it can be used for applications that uses the vector extension.
LLVM checks whether the option `riscv-v-vector-bits-min` (if specified) matches the `zvl*` extension specified.

Reviewed By: craig.topper

Differential Revision: https://reviews.llvm.org/D108694
@topperc
Copy link
Contributor

topperc commented Jan 15, 2022

Did we reach consensus on whether max_eew or elen was the right term?

The spec defines ELEN as The maximum size in bits of a vector element that any operation can produce or consume

Section 3.4.2 includes these sentences In the standard extensions, SEWMIN=8. For standard vector extensions with ELEN=32, fractional LMULs of 1/2 and 1/4 must be supported. For standard vector extensions with ELEN=64, fractional LMULs of 1/2, 1/4, and 1/8 must be supported.

I take "standard vector extensions with ELEN=32" to mean Zve32x, Zve32f.

And "standard vector extensions with ELEN=64" to mean Zve64x, Zve64f, Zve64d, and V.

So to me it feels like elen is the right term for the concept.

@kasanovic
Copy link

ELEN works for the purpose of knowing the largest size individual element. Some instructions, like SiFive matrix ops, effectively operate on larger units (e.g., 256b) but they are treated as vectors/aggregates of smaller elements, with the aggregate delimited by using larger vl rather than a larger SEW/EEW. There will be some debate as we add vector crypto on how to handle this for long crypto blocks, and not sure how that will land yet.

@kito-cheng
Copy link
Collaborator Author

Sounds like ELEN is more right term here, let me update that.

@kito-cheng
Copy link
Collaborator Author

Changes:

  • Rename __riscv_v_max_eew/__riscv_v_max_eew_fp to __riscv_v_elen/__riscv_v_elen_fp

@kito-cheng
Copy link
Collaborator Author

@aswaterman @kasanovic @nick-knight let me know if look good to you :)

@nick-knight
Copy link

@kito-cheng I don't see any harm in the current proposal, as long as we still plan to add zve* macros (perhaps in a separate PR).

@kito-cheng
Copy link
Collaborator Author

@nick-knight Sure, macro for zve* extension will added in separated PR.

@kito-cheng
Copy link
Collaborator Author

@asb @frasercrmck @topperc @rdolbeau would you mind give an explicit LGTM if that's OK to you? thanks :)

eopXD pushed a commit to llvm/llvm-project that referenced this pull request Jan 20, 2022
`zve` is the new standard vector extension to specify varying degrees of
vector support for embedding processors. The `zve` extension is related
to the `zvl` extension and other updates that are added in v1.0.

According to riscv-non-isa/riscv-c-api-doc#21,
Clang defines macro `__riscv_v_max_elen`,  `__riscv_v_max_elen_fp` for
`zve` and it can be used by applications that uses the vector extension.

Authored by: Zakk Chen <zakk.chen@sifive.com> @khchen
Co-Authored by: Eop Chen <eop.chen@sifive.com> @eopXD

Reviewed By: craig.topper

Differential Revision: https://reviews.llvm.org/D112408
@frasercrmck
Copy link

Looks good other than minor clarifications

@@ -39,6 +39,59 @@ https://creativecommons.org/licenses/by/4.0/.
| __riscv_xlen | <ul><li>32 for rv32</li><li>64 for rv64</li><li>128 for rv128</ul> | Always defined. |
| __riscv_flen | <ul><li>32 if the F extension is available **or**</li><li>64 if `D` extension available **or**</li><li>128 if `Q` extension available</li></ul> | `F` extension is available. |
| __riscv_32e | 1 | `E` extension is available. |
| __riscv_v_min_vlen | <N> (see [__riscv_v_min_vlen](#__riscv_v_min_vlen)) | The `V` extension or one of the `Zve*` extensions is available. |
| __riscv_v_elen | <N> (see [__riscv_v_elen](#__riscv_v_elen)) | The `V` extension or one of the `Zve*` extensions is available. |
| __riscv_v_elen_fp | <N> (see [__riscv_v_elen_fp](#__riscv_v_elen_fp)) | The `V` extension or one of the `Zve*` extensions is available. |
Copy link
Contributor

Choose a reason for hiding this comment

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

The when defined for __riscv_v_elen_fp isn't quite right. zve32x/zve64x don't define this.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

zve32x and zve64x still define that but defined as 0

Copy link
Contributor

Choose a reason for hiding this comment

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

Oops I missed that. Thanks.

@asb
Copy link

asb commented Jan 21, 2022

@asb @frasercrmck @topperc @rdolbeau would you mind give an explicit LGTM if that's OK to you? thanks :)

Craig and Fraser have done much more with the vector spec than I have, so I'm happy if they're happy.

@topperc
Copy link
Contributor

topperc commented Jan 21, 2022

LGTM

Introduce `__riscv_min_vlen`, `__riscv_v_elen` and `__riscv_v_elen_fp`
macro, for let programer easier to get vector related information, like:

- What's the minimal VLEN value is guaranteed current architecture
  extension?
- What's the maximal available ELEN is guaranteed current
  architecture extension?
- What's the maximal available ELEN for floating-point operation is
  guaranteed current architecture extension?

It's possible to get the information by testing serveral architecture extension
test macro, but the life can be simpler if toolchain can help, for
example, if you want to check the minimal VLEN value via those macro:

```c
 #if defined(__riscv_zvl512b)
   #define __riscv_min_vlen 512
 #elif defined(__riscv_zvl256b)
   #define __riscv_min_vlen 256
 #elif defined(__riscv_zvl128b) || defined(__riscv_v)
   #define __riscv_min_vlen 128
 #elif defined(__riscv_zvl64b) || defined(__riscv_zve64x) || \
       defined(__riscv_zve64f) || defined(__riscv_zve64d)
   #define __riscv_min_vlen 64
 #elif defined(__riscv_zvl32b) || defined(__riscv_zve32x) || \
       defined(__riscv_zve32f)
   #define __riscv_min_vlen 32
 #endif

```

And it's not scalable solution since in theory we can have up to zvl32768b.
@kito-cheng
Copy link
Collaborator Author

Gonna merge this, we got approval from GNU and LLVM community and vector ext. are ratified ext.

@kito-cheng kito-cheng merged commit aec3c3b into riscv-non-isa:master Feb 11, 2022
@kito-cheng kito-cheng deleted the v-ext-marco branch February 11, 2022 03:26
xionghul pushed a commit to xionghul/gcc that referenced this pull request Mar 21, 2022
See also:
riscv-non-isa/riscv-c-api-doc#21

gcc/ChangeLog:

	* common/config/riscv/riscv-common.cc (riscv_ext_flag_table):
	Update flag name and mask name.
	* config/riscv/riscv-c.cc (riscv_cpu_cpp_builtins): Define
	misc macro for vector extensions.
	* config/riscv/riscv-opts.h (MASK_VECTOR_EEW_32): Rename to ...
	(MASK_VECTOR_ELEN_32): ... this.
	(MASK_VECTOR_EEW_64): Rename to ...
	(MASK_VECTOR_ELEN_64): ... this.
	(MASK_VECTOR_EEW_FP_32): Rename to ...
	(MASK_VECTOR_ELEN_FP_32): ... this.
	(MASK_VECTOR_EEW_FP_64): Rename to ...
	(MASK_VECTOR_ELEN_FP_64): ... this.
	(TARGET_VECTOR_ELEN_32): New.
	(TARGET_VECTOR_ELEN_64): Ditto.
	(TARGET_VECTOR_ELEN_FP_32): Ditto.
	(TARGET_VECTOR_ELEN_FP_64): Ditto.
	(TARGET_MIN_VLEN): Ditto.
	* config/riscv/riscv.opt (riscv_vector_eew_flags): Rename to ...
	(riscv_vector_elen_flags): ... this.

gcc/testsuite/ChangeLog:

	* gcc.target/riscv/arch-13.c: New.
	* gcc.target/riscv/arch-14.c: Ditto.
	* gcc.target/riscv/arch-15.c: Ditto.
	* gcc.target/riscv/predef-18.c: Ditto.
	* gcc.target/riscv/predef-19.c: Ditto.
	* gcc.target/riscv/predef-20.c: Ditto.
RichardGaleSonos pushed a commit to RichardGaleSonos/gcc that referenced this pull request May 12, 2022
See also:
riscv-non-isa/riscv-c-api-doc#21

gcc/ChangeLog:

	* common/config/riscv/riscv-common.cc (riscv_ext_flag_table):
	Update flag name and mask name.
	* config/riscv/riscv-c.cc (riscv_cpu_cpp_builtins): Define
	misc macro for vector extensions.
	* config/riscv/riscv-opts.h (MASK_VECTOR_EEW_32): Rename to ...
	(MASK_VECTOR_ELEN_32): ... this.
	(MASK_VECTOR_EEW_64): Rename to ...
	(MASK_VECTOR_ELEN_64): ... this.
	(MASK_VECTOR_EEW_FP_32): Rename to ...
	(MASK_VECTOR_ELEN_FP_32): ... this.
	(MASK_VECTOR_EEW_FP_64): Rename to ...
	(MASK_VECTOR_ELEN_FP_64): ... this.
	(TARGET_VECTOR_ELEN_32): New.
	(TARGET_VECTOR_ELEN_64): Ditto.
	(TARGET_VECTOR_ELEN_FP_32): Ditto.
	(TARGET_VECTOR_ELEN_FP_64): Ditto.
	(TARGET_MIN_VLEN): Ditto.
	* config/riscv/riscv.opt (riscv_vector_eew_flags): Rename to ...
	(riscv_vector_elen_flags): ... this.

gcc/testsuite/ChangeLog:

	* gcc.target/riscv/arch-13.c: New.
	* gcc.target/riscv/arch-14.c: Ditto.
	* gcc.target/riscv/arch-15.c: Ditto.
	* gcc.target/riscv/predef-18.c: Ditto.
	* gcc.target/riscv/predef-19.c: Ditto.
	* gcc.target/riscv/predef-20.c: Ditto.
mem-frob pushed a commit to draperlaboratory/hope-llvm-project that referenced this pull request Oct 7, 2022
`zvl` is the new standard vector extension that specifies the minimum vector length of the vector extension.
The `zvl` extension is related to the `zve` extension and other updates that are added in v1.0.

According to riscv-non-isa/riscv-c-api-doc#21,
Clang defines macro `__riscv_v_min_vlen` for `zvl` and it can be used for applications that uses the vector extension.
LLVM checks whether the option `riscv-v-vector-bits-min` (if specified) matches the `zvl*` extension specified.

Reviewed By: craig.topper

Differential Revision: https://reviews.llvm.org/D108694
mem-frob pushed a commit to draperlaboratory/hope-llvm-project that referenced this pull request Oct 7, 2022
`zve` is the new standard vector extension to specify varying degrees of
vector support for embedding processors. The `zve` extension is related
to the `zvl` extension and other updates that are added in v1.0.

According to riscv-non-isa/riscv-c-api-doc#21,
Clang defines macro `__riscv_v_max_elen`,  `__riscv_v_max_elen_fp` for
`zve` and it can be used by applications that uses the vector extension.

Authored by: Zakk Chen <zakk.chen@sifive.com> @khchen
Co-Authored by: Eop Chen <eop.chen@sifive.com> @eopXD

Reviewed By: craig.topper

Differential Revision: https://reviews.llvm.org/D112408
pz9115 pushed a commit to pz9115/revyos-gcc that referenced this pull request Oct 22, 2023
See also:
riscv-non-isa/riscv-c-api-doc#21

gcc/ChangeLog:

	* common/config/riscv/riscv-common.cc (riscv_ext_flag_table):
	Update flag name and mask name.
	* config/riscv/riscv-c.cc (riscv_cpu_cpp_builtins): Define
	misc macro for vector extensions.
	* config/riscv/riscv-opts.h (MASK_VECTOR_EEW_32): Rename to ...
	(MASK_VECTOR_ELEN_32): ... this.
	(MASK_VECTOR_EEW_64): Rename to ...
	(MASK_VECTOR_ELEN_64): ... this.
	(MASK_VECTOR_EEW_FP_32): Rename to ...
	(MASK_VECTOR_ELEN_FP_32): ... this.
	(MASK_VECTOR_EEW_FP_64): Rename to ...
	(MASK_VECTOR_ELEN_FP_64): ... this.
	(TARGET_VECTOR_ELEN_32): New.
	(TARGET_VECTOR_ELEN_64): Ditto.
	(TARGET_VECTOR_ELEN_FP_32): Ditto.
	(TARGET_VECTOR_ELEN_FP_64): Ditto.
	(TARGET_MIN_VLEN): Ditto.
	* config/riscv/riscv.opt (riscv_vector_eew_flags): Rename to ...
	(riscv_vector_elen_flags): ... this.

gcc/testsuite/ChangeLog:

	* gcc.target/riscv/arch-13.c: New.
	* gcc.target/riscv/arch-14.c: Ditto.
	* gcc.target/riscv/arch-15.c: Ditto.
	* gcc.target/riscv/predef-18.c: Ditto.
	* gcc.target/riscv/predef-19.c: Ditto.
	* gcc.target/riscv/predef-20.c: Ditto.
yulong18 pushed a commit to yulong18/ruyisdk-gcc that referenced this pull request Feb 29, 2024
See also:
riscv-non-isa/riscv-c-api-doc#21

gcc/ChangeLog:

	* common/config/riscv/riscv-common.cc (riscv_ext_flag_table):
	Update flag name and mask name.
	* config/riscv/riscv-c.cc (riscv_cpu_cpp_builtins): Define
	misc macro for vector extensions.
	* config/riscv/riscv-opts.h (MASK_VECTOR_EEW_32): Rename to ...
	(MASK_VECTOR_ELEN_32): ... this.
	(MASK_VECTOR_EEW_64): Rename to ...
	(MASK_VECTOR_ELEN_64): ... this.
	(MASK_VECTOR_EEW_FP_32): Rename to ...
	(MASK_VECTOR_ELEN_FP_32): ... this.
	(MASK_VECTOR_EEW_FP_64): Rename to ...
	(MASK_VECTOR_ELEN_FP_64): ... this.
	(TARGET_VECTOR_ELEN_32): New.
	(TARGET_VECTOR_ELEN_64): Ditto.
	(TARGET_VECTOR_ELEN_FP_32): Ditto.
	(TARGET_VECTOR_ELEN_FP_64): Ditto.
	(TARGET_MIN_VLEN): Ditto.
	* config/riscv/riscv.opt (riscv_vector_eew_flags): Rename to ...
	(riscv_vector_elen_flags): ... this.

gcc/testsuite/ChangeLog:

	* gcc.target/riscv/arch-13.c: New.
	* gcc.target/riscv/arch-14.c: Ditto.
	* gcc.target/riscv/arch-15.c: Ditto.
	* gcc.target/riscv/predef-18.c: Ditto.
	* gcc.target/riscv/predef-19.c: Ditto.
	* gcc.target/riscv/predef-20.c: Ditto.
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.

None yet