Skip to content

Use BLST portable feature to enable runtime ADX instruction set check #1630

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

Merged
merged 5 commits into from
Apr 11, 2024

Conversation

Alenar
Copy link
Collaborator

@Alenar Alenar commented Apr 10, 2024

Content

This PR changes the mithril-stm dependencies in order to always enforce the portable features of BLST.
This enable a runtime check of the ADX instruction set instead of the compile time check that we used before, allowing users with older CPUs to use our compiled binaries instead of needing to compile them themselves.

Making this changes make the mithril-stm portable feature, and all equivalent features in all our crates, useless as it now do strictly nothing.

For compatibility purpose, since those crates are published to crates.io, the portable feature is kept on mithril-stm and mithril-client but is removed on the other crates (the CI have been adapted for this change).

We should communicate this change to advertise users that depends on our crates.io published crates that this feature will be removed soon.

Pre-submit checklist

  • Branch
    • Crates versions are updated (if relevant)
    • CHANGELOG file is updated (if relevant)
    • Commit sequence broadly makes sense
    • Key commits have useful messages
  • PR
    • No clippy warnings in the CI
    • Self-reviewed the diff
    • Useful pull request description
    • Reviewer requested
  • Documentation
    • Update README file (if relevant)
    • Update documentation website (if relevant)
    • Add dev blog post (if relevant)

Issue(s)

Relates to #1614

Copy link

github-actions bot commented Apr 10, 2024

Test Results

    3 files  ±0     42 suites  ±0   8m 31s ⏱️ -23s
  914 tests ±0    914 ✅ ±0  0 💤 ±0  0 ❌ ±0 
1 008 runs  ±0  1 008 ✅ ±0  0 💤 ±0  0 ❌ ±0 

Results for commit 7ad18ee. ± Comparison against base commit f4cafd6.

♻️ This comment has been updated with latest results.

@Alenar Alenar temporarily deployed to testing-preview April 10, 2024 16:48 — with GitHub Actions Inactive
@Alenar Alenar temporarily deployed to testing-sanchonet April 10, 2024 16:48 — with GitHub Actions Inactive
@Alenar Alenar force-pushed the djo/1614/blst_portable_as_default branch from e56d8e4 to cb0fc8c Compare April 11, 2024 09:17
@Alenar Alenar temporarily deployed to testing-preview April 11, 2024 09:23 — with GitHub Actions Inactive
@Alenar Alenar temporarily deployed to testing-sanchonet April 11, 2024 09:23 — with GitHub Actions Inactive
Copy link
Member

@jpraynaud jpraynaud left a comment

Choose a reason for hiding this comment

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

LGTM 👍

Alenar added 3 commits April 11, 2024 15:03
As blst now check for Intel ADX instruction set at runtime if this
feature is enabled (otherwise there's no runtime check).
As it's no longer needed since it does nothing now.
@Alenar Alenar force-pushed the djo/1614/blst_portable_as_default branch 3 times, most recently from 591d0b2 to c91aeff Compare April 11, 2024 13:36
@Alenar Alenar force-pushed the djo/1614/blst_portable_as_default branch from c91aeff to 7ad18ee Compare April 11, 2024 13:38
@Alenar Alenar temporarily deployed to testing-preview April 11, 2024 13:55 — with GitHub Actions Inactive
@Alenar Alenar temporarily deployed to testing-sanchonet April 11, 2024 13:55 — with GitHub Actions Inactive
@Alenar Alenar merged commit d3064e6 into main Apr 11, 2024
41 checks passed
@Alenar Alenar deleted the djo/1614/blst_portable_as_default branch April 11, 2024 13:56
@Alenar Alenar mentioned this pull request May 17, 2024
2 tasks
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.

3 participants