-
Notifications
You must be signed in to change notification settings - Fork 13.4k
Need comprehensive story for target_feature
compat
#140570
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
Comments
Most concerningly, relying on "let LLVM take us down if we do something foolish" doesn't always work and sometimes LLVM will happily emit horrifically invalid code. |
I'll summarize some of the RISC-V requirements: Must be ImplementedConflict CheckingFor instance, the F extension ( Also see the section "Custom Requirement Checking" below. Should be ImplementedExtension GroupFor instance, the A extension consists of two subsets: the Zalrsc and Zaamo extensions ( Reverse implication of an extension group is not described in RISC-V specifications but feature-based extension handling (i.e. if we have a feature, we have an extension; even if the specification does not specify extension dependencies) is more consistent and should be used on the language-end such as Rust and even LLVM (not Clang side but lower-end LLVM side) implements this (on the other hand, GNU Binutils largely follows the specifications, not implementing the extension groups). That's why I did implement extension groups in the runtime feature detection logic for RISC-V (rust-lang/stdarch#1770) but didn't include documentation about extension groups in rust-lang/stdarch#1779 (section: Note: About Reverse Implication (Extension Groups); reverted by rust-lang/stdarch#1792 and partially restored in rust-lang/stdarch#1797) because documenting behavior of extension groups (currently not implemented in Rust's feature handling system via One of the ways to implement this is to implement Multiple Requirements I'll describe below. Multiple RequirementsSome RISC-V extensions require multiple extensions to be implied (while current Rust's feature handling system only implements implication from a target feature). Currently, following two extensions require such handling and I excluded them from RISC-V feature addition PRs (as I explained in #139440):
Reverse implication of an extension group can be implemented like this:
Optional but RecommendedLLVM Codegen (at least): Additional Implication on Rust side after LLVM side QueryingIn the LLVM Codegen, it first enables (default) target features by querying enabled features in LLVM (that itself is okay) but does not run implication logic after querying. If LLVM does not keep track of all relations of supported extensions, Rust will fail to enable specific target features (sorry, I strongly remember that I encountered a problem related to this but forgot which combination caused To deal with this, I recommend running implication after querying LLVM-enabled target features. The same would apply to other codegen backends. OptionalCustom Requirement CheckingIn GNU Binutils, Zvl* extensions (denoting minimum vector register length) require at least one of the vector extension subsets (either full-set V or Zve* subsets). I think this is not that important on the Rust side but if we prefer, implementing target feature checking (as well as conflict checking)
would be nice. |
Please explain jargon-y acronyms before using them. :) |
@RalfJung Sorry, that's correct. I fixed the post above. |
Supplement: Feature-based Extension HandlingAlthough that this is not completely irrelevant to the main subject, this is not important (here) compared to the post above, I note about that as the separate supplemental post. This concept (the words I used above) means, handling existence of an extension strictly as a set of features implemented. RISC-V defines numerous extensions and some of them are
I'll provide an example: The Zvknha and Zvknhb extensions implement accelerator instructions for the SHA-2 hashing functions (32-bit integer based SHA-256, 64-bit integer based SHA-512, and their derivatives like SHA-256→SHA-224 and SHA-512→SHA-512/224 (both shorter 224-bit hashing functions but computation part in the middle can be shared with corresponding bases)).
They completely share the instructions (the only difference is vector configuration prior to shared SHA-2 processing instructions: SHA-256 if EEW1=SEW2=32 and SHA-512 if EEW=SEW=64). That means, the Zvknhb extension (SHA-256 and SHA-512) is functionally a strict superset of the Zvknha extension (SHA-256 only); i.e. all implementations with the Zvknhb extension effectively has the Zvknha extension. Currently, both LLVM and GNU toolchains chose not to imply the Zvknha extension from the Zvknhb extension. However, on the language end (at least), this is inconvenient (would require like
With that PR, the Footnotes
|
From #138872 per Amanieu:
We should be emitting an error way before anything gets to LLVM because we have to have a better idea of what features are actually compatible. We effectively need to have both global and local notions of it.
The text was updated successfully, but these errors were encountered: