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.

Sign up for GitHub

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

Jump to bottom

Implement LLVM x86 bmi intrinsics #3674

Merged
merged 1 commit into from
Jun 20, 2024
Merged

Implement LLVM x86 bmi intrinsics #3674

merged 1 commit into from
Jun 20, 2024

Conversation

TDecking
Copy link
Contributor

This implements the intrinsics for both the bmi1 and bmi2 ISA extensions. All of these intrinsics live inside the same namespace as far as LLVM is concerned, which is why it is arguably better to bundle the implementations of these two extensions.

@folkertdev folkertdev mentioned this pull request Jun 15, 2024
Closed
@TDecking
Copy link
Contributor Author

@rustbot ready

@rustbot rustbot added the S-waiting-on-review Status: Waiting for a review to complete label Jun 17, 2024
RalfJung
RalfJung reviewed Jun 18, 2024
View reviewed changes
Copy link
Member

@RalfJung RalfJung left a comment

Choose a reason for hiding this comment

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

Thanks!
The world of Intel intrinsics is quite bizarre form an outside perspective. ;)

src/shims/x86/bmi.rs Outdated Show resolved Hide resolved
src/shims/x86/bmi.rs Outdated Show resolved Hide resolved
tests/pass/shims/x86/intrinsics-x86-bmi.rs Show resolved Hide resolved
tests/pass/shims/x86/intrinsics-x86-bmi.rs Show resolved Hide resolved
src/shims/x86/bmi.rs Show resolved Hide resolved
@rustbot
Copy link
Collaborator

rustbot commented Jun 19, 2024

There are merge commits (commits with multiple parents) in your changes. We have a no merge policy so these commits will need to be removed for this pull request to be merged.

You can start a rebase with the following commands:

$ # rebase
$ git pull --rebase https://github.com/rust-lang/miri.git master
$ git push --force-with-lease

The following commits are merge commits:

RalfJung
RalfJung reviewed Jun 19, 2024
View reviewed changes
src/shims/x86/bmi.rs Outdated Show resolved Hide resolved
@TDecking
Copy link
Contributor Author

@RalfJung All the comments are added.

@RalfJung
Copy link
Member

Thanks, looks good! Please squash the commits.

@RalfJung RalfJung added S-waiting-on-author Status: Waiting for the PR author to address review comments and removed S-waiting-on-review Status: Waiting for a review to complete labels Jun 20, 2024
@TDecking
Copy link
Contributor Author

Everything is ready.

@TDecking
Copy link
Contributor Author

@rustbot ready

@rustbot rustbot added S-waiting-on-review Status: Waiting for a review to complete and removed S-waiting-on-author Status: Waiting for the PR author to address review comments labels Jun 20, 2024
@RalfJung
Copy link
Member

Awesome, thanks for the PR and the adjustments during review. :)

@bors r+

@bors
Copy link
Collaborator

bors commented Jun 20, 2024

📌 Commit 459eada has been approved by RalfJung

It is now in the queue for this repository.

@bors
Copy link
Collaborator

bors commented Jun 20, 2024

⌛ Testing commit 459eada with merge cc9699d...

@bors
Copy link
Collaborator

bors commented Jun 20, 2024

☀️ Test successful - checks-actions
Approved by: RalfJung
Pushing cc9699d to master...

@bors bors merged commit cc9699d into rust-lang:master Jun 20, 2024
8 checks passed
@TDecking TDecking deleted the bmi branch June 20, 2024 12:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@RalfJung RalfJung RalfJung left review comments

@folkertdev folkertdev folkertdev left review comments

Assignees
No one assigned
Labels
S-waiting-on-review Status: Waiting for a review to complete
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@TDecking @rustbot @RalfJung @bors @folkertdev