std_detect: RISC-V: Best effort implication of target features by custom cfg by a4lg · Pull Request #145810 · rust-lang/rust (original) (raw)
This PR implements new feature to std_detect's feature! macro: implication of target features by custom cfg and then uses this to implement static implication logic on RISC-V where the standard target feature handling does not capture all the cases.
This PR implements implication logic for three kinds of target features:
- Group extensions where each equals aggregation of its sub-extensions (exact)
- Complex implications around the "C" extension (in a best effort manner)
For detailed intent of the changes, see each commit message.
It also changes some AArch64 code to use the new syntax added in this PR and removes the old custom syntax to imply a target feature.
cf. Z3-based prover for complex implications around the "C" extension
Draft Design Consideration 1: RISC-V: How to handle ISA Bases?
Implementing custom implications to base ISA target features like rv32i might be an option but I have a concern (which will not happen for now but plausible in the future).
In particular, I have a concern on the following case: the program is built for an rv32e target but ran on an rv32i environment (which is a superset of rv32e) and reports existence of rv32i. If that happens, both rv32e and rv32i would return true on std::arch::is_riscv_feature_detected!.
So, the commit implementing implication of base ISA target features (originally commit 3) was marked DRAFT.
My original position: Slightly Negative.
Conclusion: Remove original commit 3 and do not imply ISA bases.
Draft Design Consideration 2: Substituting implied by target_features:
This PR (originally commit 2) implements implication by custom cfg with following syntax: implied by cfg(CFG);.
Unlike existing implied by target_features: [A, B, ...]; (which will work like cfg(any(target_feature = "A", target_feature = "B", ...))), this syntax accepts any cfgs.
Interestingly, implied by target_features: is currently only used on AArch64 and in the simplest form (implied from one target feature). So if there's no good reason keeping this, we have an option to remove implied by target_features: [...]; entirely and substitute with implied by cfg(...);.
Commits that make such changes (originally commits 6 and 7) were marked DRAFT.
My original position: Positive (but leaving draft after hearing from others).
Conclusion: Substitute as the original.