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

Implement simplified backend selection #428

Merged
merged 3 commits into from
Nov 13, 2022

Conversation

tarcieri
Copy link
Contributor

@tarcieri tarcieri commented Nov 11, 2022

As proposed in #414, this commit changes the backend selection approach, introspecting target_pointer_width to select u32_backend vs u64_backend (or fiat_u32_backend/fiat_u64_backend if the fiat_backend feature is enabled).

This helps eliminate the use of non-additive features, and also the rather confusing errors that happen if multiple backends are selected (i.e. thousands of lines of rustc errors).

The selection logic checks if target_pointer_width = "64" and uses the 64-bit backend, or falls back to the 32-bit backend otherwise. This means the crate will always have a valid backend regardless of the pointer width, although there may be odd edge cases for exotic platforms which would optimally use the 64-bit backend but have a non-"64" target pointer width for whatever reason. We can handle those cases as they come up.

@tarcieri tarcieri requested a review from rozbb November 11, 2022 02:54
@tarcieri
Copy link
Contributor Author

Note: I deliberately punted on trying to change fiat_backend and simd_backend into a cfg attribute to keep this PR relatively simple

Cargo.toml Show resolved Hide resolved
README.md Show resolved Hide resolved
@tarcieri tarcieri force-pushed the simplified-backend-selection branch 2 times, most recently from 40bac3c to 935f182 Compare November 11, 2022 03:23
@pinkforest pinkforest mentioned this pull request Nov 11, 2022
Copy link
Contributor

@rozbb rozbb left a comment

Choose a reason for hiding this comment

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

Looking good! And nice changes to the CI. Left some small comments here and there. After that, and a conflict resolution, this is good to go.

README.md Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
src/lib.rs Outdated Show resolved Hide resolved
As proposed in #414, this commit changes the backend selection approach,
introspecting `target_pointer_width` to select `u32_backend` vs
`u64_backend` (or `fiat_u32_backend`/`fiat_u64_backend` if the
`fiat_backend` feature is enabled).

This helps eliminate the use of non-additive features, and also the
rather confusing errors that happen if multiple backends are selected
(i.e. thousands of lines of rustc errors).

The selection logic checks if `target_pointer_width = "64"` and uses the
64-bit backend, or falls back to the 32-bit backend otherwise. This
means the crate will always have a valid backend regardless of the
pointer width, although there may be odd edge cases for exotic platforms
which would optimally use the 64-bit backend but have a non-"64" target
pointer width for whatever reason. We can handle those cases as they
come up.
@rozbb rozbb merged commit 081f632 into release/4.0 Nov 13, 2022
@tarcieri tarcieri deleted the simplified-backend-selection branch November 13, 2022 23:46
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.

2 participants