Fix some more uses of LLVM intrinsics by sayantn · Pull Request #1820 · rust-lang/stdarch (original) (raw)
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 andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Conversation6 Commits5 Checks62 Files changed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
[ Show hidden characters]({{ revealButtonHref }})
- Use rust intrinsics for more ARM intrinsics
- Fix incorrect intrinsic names (these were autoupgraded by LLVM, so didn't result in invalid IR)
- Remove uses of deprecated type-specific pointers, LLVM only supports opaque pointers now.
@folkertdev is it okay to replace the llvm.{u,s}{max,min} calls with something like simd_select(simd_gt(a, b), a, b))? I am just worried if it will optimize well
edit: seems like it is not a problem actually https://godbolt.org/z/6r1javWc6
r? @Amanieu
rustbot has assigned @Amanieu.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.
Use r? to explicitly pick a reviewer
Just try it, you can check whether that works by running the docker container. We should be testing for the specific instruction (adjust for your target):
TARGET=aarch64-unknown-linux-gnu sh ci/run-docker.sh aarch64-unknown-linux-gnu
If it doesn't work, let me know because we should get that fixed in LLVM.
@folkertdev is it okay to replace the
llvm.{u,s}{max,min}calls with something likesimd_select(simd_gt(a, b), a, b))? I am just worried if it will optimize well
No, those 2 are not equivalent, specifically with regards to negative zero and NaN.
umax and smax are only for integers right? For floats the expressions are indeed not equivalent in some nasty ways.
The fact that LLVM generates the same code for both suggests a bug in the LLVM backend.
Nevermind, it's fine if this is only for integers.
This comment was marked as resolved.
Merged via the queue into rust-lang:master with commit 86c8496
62 checks passed
bors added a commit to rust-lang/rust that referenced this pull request
github-actions bot pushed a commit to rust-lang/miri that referenced this pull request
bjorn3 pushed a commit to rust-lang/rustc_codegen_cranelift that referenced this pull request
github-actions bot pushed a commit to rust-lang/rustc-dev-guide that referenced this pull request
lnicola pushed a commit to lnicola/rust-analyzer that referenced this pull request
tautschnig pushed a commit to model-checking/verify-rust-std that referenced this pull request