-
Notifications
You must be signed in to change notification settings - Fork 12.7k
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
Add simd math intrinsics and gather/scatter #50521
Conversation
(rust_highfive has picked a reviewer for you, use r? to override) |
Still needs to be cleaned up and need to add run-time tests, but I'd like initial feedback on the general approach. |
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
@petrochenkov all tests are there |
Nice! For the math intrinsics though, those can all be defined natively with |
// except according to those terms. | ||
|
||
// ignore-emscripten | ||
// error-pattern: panicked |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this be removed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think so, thanks.
src/librustc_trans/intrinsic.rs
Outdated
} | ||
} | ||
|
||
fn llvm_vector_ty(cx: &CodegenCx, elem_ty: ty::Ty, vec_len: usize, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks similar to the above definition I think? Could they be shared?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes those are indeed identical, thanks.
Yes, the ones that I've added here can all be defined with
My plan was to submit a sub-sequent PR that adds the "approximative"/"estimate" versions of the intrinsics, since those are also widely supported on most architectures (approximative sqrt, sin and cos are widely supported at least). We need to set some math flags like So that part at least must be done in rustc. I might have time to add them this weekend, so if you prefer I can add them to this PR so that we can see the whole picture. |
@eddyb for the gather/scatter intrinsics, I've kept it simple for now, but I'll try to add the llvm functions that generate the function names to rustllvm and use those instead. Some of the tests failed with llvm 3.9 because
which is surprising to me because I did not know that LLVM intrinsics name syntax changed at some point in time. I hope that it doesn't change again in a way that forces us to rename all |
@gnzlbg oh it's fine if stdsimd only supports the latest LLVM currently, and so most of the tests are fine to just ignore on non-HEAD LLVM. If the math intrinsics can be defined with |
(I'd prefer to only put in the compiler what must be there for one reason or another) |
Sure, I’ll just turn these ones into the approximative ones then.
|
@alexcrichton I've made the intrinsics "approximative". Given that it was a one line change, I am still unsure of whether it wouldn't be better to just handle both cases in rustc. |
Ah ok I see! Yeah if these end up looking like the "fast math" intrinsics we have it seems fine by me to land. In that case... @bors: r+ |
📌 Commit 6532c48 has been approved by |
Add simd math intrinsics and gather/scatter This PR adds simd math intrinsics for floating-point vectors (sqrt, sin, cos, pow, exp, log, fma, abs, etc.) and the generic simd gather/scatter intrinsics.
fn simd_fma<T>(x: T, y: T, z: T) -> T; | ||
fn simd_flog<T>(x: T) -> T; | ||
fn simd_flog10<T>(x: T) -> T; | ||
fn simd_flog2<T>(x: T) -> T; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[01:29:38] ---- [run-pass] run-pass/simd-intrinsic-float-math.rs stdout ----
[01:29:38]
[01:29:38] error: compilation failed!
[01:29:38] status: exit code: 101
[01:29:38] command: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/bin/rustc" "/checkout/src/test/run-pass/simd-intrinsic-float-math.rs" "-L" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/run-pass" "--target=arm-linux-androideabi" "-C" "prefer-dynamic" "-o" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/run-pass/simd-intrinsic-float-math.stage2-arm-linux-androideabi" "-Crpath" "-O" "-Zunstable-options" "-Lnative=/checkout/obj/build/arm-linux-androideabi/native/rust-test-helpers" "-Clinker=/android/ndk/arm-14/bin/arm-linux-androideabi-clang" "-L" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/run-pass/simd-intrinsic-float-math.stage2-arm-linux-androideabi.aux"
[01:29:38] stdout:
[01:29:38] ------------------------------------------
[01:29:38]
[01:29:38] ------------------------------------------
[01:29:38] stderr:
[01:29:38] ------------------------------------------
[01:29:38] error: linking with `/android/ndk/arm-14/bin/arm-linux-androideabi-clang` failed: exit code: 1
[01:29:38] |
[01:29:38] = note: "/android/ndk/arm-14/bin/arm-linux-androideabi-clang" "-Wl,--as-needed" "-Wl,-z,noexecstack" "-Wl,--allow-multiple-definition" "-L" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/lib/rustlib/arm-linux-androideabi/lib" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/run-pass/simd-intrinsic-float-math.simd_intrinsic_float_math0-317d481089b8c8fe83113de504472633.rs.rcgu.o" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/run-pass/simd-intrinsic-float-math.simd_intrinsic_float_math1-317d481089b8c8fe83113de504472633.rs.rcgu.o" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/run-pass/simd-intrinsic-float-math.simd_intrinsic_float_math2-317d481089b8c8fe83113de504472633.rs.rcgu.o" "-o" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/run-pass/simd-intrinsic-float-math.stage2-arm-linux-androideabi" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/run-pass/simd-intrinsic-float-math.crate.allocator.rcgu.o" "-Wl,--gc-sections" "-pie" "-Wl,-z,relro,-z,now" "-Wl,-O1" "-nodefaultlibs" "-L" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/run-pass" "-L" "/checkout/obj/build/arm-linux-androideabi/native/rust-test-helpers" "-L" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/run-pass/simd-intrinsic-float-math.stage2-arm-linux-androideabi.aux" "-L" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/lib/rustlib/arm-linux-androideabi/lib" "-Wl,--start-group" "-L" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/lib/rustlib/arm-linux-androideabi/lib" "-l" "std-dd7e1cb952f01bfe" "-Wl,--end-group" "-Wl,-Bstatic" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/lib/rustlib/arm-linux-androideabi/lib/libcompiler_builtins-93fbfd71748200b0.rlib" "-Wl,-Bdynamic" "-l" "dl" "-l" "log" "-l" "gcc" "-l" "gcc" "-l" "c" "-l" "m" "-Wl,-rpath,$ORIGIN/../../stage2/lib/rustlib/arm-linux-androideabi/lib" "-Wl,-rpath,/checkout/obj/lib/rustlib/arm-linux-androideabi/lib" "-Wl,--enable-new-dtags"
[01:29:38] = note: /checkout/obj/build/x86_64-unknown-linux-gnu/test/run-pass/simd-intrinsic-float-math.simd_intrinsic_float_math1-317d481089b8c8fe83113de504472633.rs.rcgu.o:simd_intrinsic_float_math1-317d481089b8c8fe83113de504472633.rs:function simd_intrinsic_float_math::main::h3154db08c8edf39b: error: undefined reference to 'log2f'
[01:29:38] /checkout/obj/build/x86_64-unknown-linux-gnu/test/run-pass/simd-intrinsic-float-math.simd_intrinsic_float_math1-317d481089b8c8fe83113de504472633.rs.rcgu.o:simd_intrinsic_float_math1-317d481089b8c8fe83113de504472633.rs:function simd_intrinsic_float_math::main::h3154db08c8edf39b: error: undefined reference to 'log2f'
[01:29:38] /checkout/obj/build/x86_64-unknown-linux-gnu/test/run-pass/simd-intrinsic-float-math.simd_intrinsic_float_math1-317d481089b8c8fe83113de504472633.rs.rcgu.o:simd_intrinsic_float_math1-317d481089b8c8fe83113de504472633.rs:function simd_intrinsic_float_math::main::h3154db08c8edf39b: error: undefined reference to 'log2f'
[01:29:38] /checkout/obj/build/x86_64-unknown-linux-gnu/test/run-pass/simd-intrinsic-float-math.simd_intrinsic_float_math1-317d481089b8c8fe83113de504472633.rs.rcgu.o:simd_intrinsic_float_math1-317d481089b8c8fe83113de504472633.rs:function simd_intrinsic_float_math::main::h3154db08c8edf39b: error: undefined reference to 'log2f'
[01:29:38] clang50: error: linker command failed with exit code 1 (use -v to see invocation)
[01:29:38]
[01:29:38]
[01:29:38] error: aborting due to previous error
[01:29:38]
[01:29:38]
[01:29:38] ------------------------------------------
[01:29:38]
[01:29:38] thread '[run-pass] run-pass/simd-intrinsic-float-math.rs' panicked at 'explicit panic', tools/compiletest/src/runtest.rs:3033:9
[01:29:38] note: Run with `RUST_BACKTRACE=1` for a backtrace.
@bors r- The new test failed on |
☔ The latest upstream changes (presumably #50893) made this pull request unmergeable. Please resolve the merge conflicts. |
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
Ping from triage @alexcrichton! This PR needs your review. |
@bors: r+ er oops, sorry this fell off my radar! |
📌 Commit de60483 has been approved by |
Add simd math intrinsics and gather/scatter This PR adds simd math intrinsics for floating-point vectors (sqrt, sin, cos, pow, exp, log, fma, abs, etc.) and the generic simd gather/scatter intrinsics.
☀️ Test successful - status-appveyor, status-travis |
This PR adds simd math intrinsics for floating-point vectors (sqrt, sin, cos, pow, exp, log, fma, abs, etc.) and the generic simd gather/scatter intrinsics.