implement minmax intrinsics #49249

gnzlbg · 2018-03-21T20:52:20Z

This adds the simd_{fmin,fmax} intrinsics, which do a vertical (lane-wise) min/max for floating point vectors that's equivalent to Rust's min/max for f32/f64.

It might make sense to make {f32,f64}::{min,max} use the minnum and minmax intrinsics as well.

HELP: I need some help with these. Either I should go to sleep or there must be something that I must be missing. AFAICT I am calling the maxnum builder correctly, yet rustc/LLVM seem to insert a call to llvm.minnum there instead... EDIT: Rust's LLVM version is too old :/

rust-highfive · 2018-03-21T20:52:30Z

r? @nikomatsakis

(rust_highfive has picked a reviewer for you, use r? to override)

gnzlbg · 2018-03-21T20:56:43Z

I am not insane! The LLVM source code looked correct, but a blame revealed that this was actually fixed in LLVM trunk 29 days ago and that it was broken before that: llvm-mirror/llvm@e271f8c

gnzlbg · 2018-03-21T21:06:15Z

@alexcrichton we could provide a correct implementation in stdsimd without these intrinsics and update those once we update to the next LLVM version. In the meantime, that might generate pretty horrible code. An alternative might be to backport that patch to our LLVM version.

alexcrichton · 2018-03-21T22:09:45Z

Ah yeah sounds like we should provide unstable versions in stdsimd with suboptimal codegen for now?

gnzlbg · 2018-03-21T22:35:19Z

I'll update this PR to make maxnum a run-fail test so that when LLVM is updated it automatically pases.

gnzlbg · 2018-03-22T08:52:14Z

So:

I've filled a rust-lang/rust issue to track this: LLVM's maxnum builder is broken in LLVM6 #49261
Added the run-fail tests: these will start passing automatically once we move to LLVM 7
Added a codegen test for minnum, commented out the codegen for maxnum - that will need to be updated once we move to LLVM 7

alexcrichton · 2018-03-22T14:13:12Z

@bors: r+

bors · 2018-03-22T14:13:13Z

📌 Commit 88268d7 has been approved by alexcrichton

kennytm · 2018-03-22T15:25:22Z

@bors r-

Unreachable expression in rustc_trans.

[00:20:22] error: unreachable expression
[00:20:22]    --> librustc_trans/builder.rs:918:13
[00:20:22]     |
[00:20:22] 918 |             instr
[00:20:22]     |             ^^^^^
[00:20:22]     |
[00:20:22] note: lint level defined here
[00:20:22]    --> librustc_trans/lib.rs:20:9
[00:20:22]     |
[00:20:22] 20  | #![deny(warnings)]
[00:20:22]     |         ^^^^^^^^
[00:20:22]     = note: #[deny(unreachable_code)] implied by #[deny(warnings)]
[00:20:22] 
[00:20:22] error: unreachable expression
[00:20:22]    --> librustc_trans/builder.rs:926:13
[00:20:22]     |
[00:20:22] 926 |             instr
[00:20:22]     |             ^^^^^
[00:20:22] 
[00:20:24] error: aborting due to 2 previous errors
[00:20:24] 
[00:20:24] error: Could not compile `rustc_trans`.

kennytm · 2018-03-22T15:37:05Z

@bors r=alexcrichton

bors · 2018-03-22T15:37:06Z

📌 Commit 0118a65 has been approved by alexcrichton

kennytm · 2018-03-22T16:44:05Z

@bors r-

CI is still failing 😞. The new test requires // min-llvm-version 6.0, and maybe // ignore-emscripten too.

[00:52:50] ---- [run-fail] run-fail/simd-intrinsic-float-minmax.rs stdout ----
[00:52:50] 	
[00:52:50] error: compilation failed!
[00:52:50] status: exit code: 101
[00:52:50] command: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/bin/rustc" "/checkout/src/test/run-fail/simd-intrinsic-float-minmax.rs" "-L" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/run-fail" "--target=x86_64-unknown-linux-gnu" "-C" "prefer-dynamic" "-o" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/run-fail/simd-intrinsic-float-minmax.stage2-x86_64-unknown-linux-gnu" "-Crpath" "-O" "-Zmiri" "-Zunstable-options" "-Lnative=/checkout/obj/build/x86_64-unknown-linux-gnu/native/rust-test-helpers" "-L" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/run-fail/simd-intrinsic-float-minmax.stage2-x86_64-unknown-linux-gnu.aux"
[00:52:50] stdout:
[00:52:50] ------------------------------------------
[00:52:50] 
[00:52:50] ------------------------------------------
[00:52:50] stderr:
[00:52:50] ------------------------------------------
[00:52:50] error: internal compiler error: librustc_trans/builder.rs:918: LLVMRustBuildMinNum is not available in LLVM version < 6.0
[00:52:50] 
[00:52:50] thread 'rustc' panicked at 'Box<Any>', librustc_errors/lib.rs:543:9
[00:52:50] note: Run with `RUST_BACKTRACE=1` for a backtrace.
[00:52:50] 
[00:52:50] note: the compiler unexpectedly panicked. this is a bug.
[00:52:50] 
[00:52:50] note: we would appreciate a bug report: https://github.com/rust-lang/rust/blob/master/CONTRIBUTING.md#bug-reports
[00:52:50] 
[00:52:50] note: rustc 1.26.0-dev running on x86_64-unknown-linux-gnu
[00:52:50] 
[00:52:50] note: compiler flags: -Z miri -Z unstable-options -C prefer-dynamic -C rpath
[00:52:50] 
[00:52:50] 
[00:52:50] ------------------------------------------
[00:52:50] 
[00:52:50] thread '[run-fail] run-fail/simd-intrinsic-float-minmax.rs' panicked at 'explicit panic', tools/compiletest/src/runtest.rs:2903:9
[00:52:50] 
[00:52:50] 
[00:52:50] failures:
[00:52:50]     [run-fail] run-fail/simd-intrinsic-float-minmax.rs

alexcrichton · 2018-03-22T17:25:18Z

@bors: r+

bors · 2018-03-22T17:25:18Z

📌 Commit d87b403 has been approved by alexcrichton

implement minmax intrinsics This adds the `simd_{fmin,fmax}` intrinsics, which do a vertical (lane-wise) `min`/`max` for floating point vectors that's equivalent to Rust's `min`/`max` for `f32`/`f64`. It might make sense to make `{f32,f64}::{min,max}` use the `minnum` and `minmax` intrinsics as well. --- ~~HELP: I need some help with these. Either I should go to sleep or there must be something that I must be missing. AFAICT I am calling the `maxnum` builder correctly, yet rustc/LLVM seem to insert a call to `llvm.minnum` there instead...~~ EDIT: Rust's LLVM version is too old :/

alexcrichton · 2018-03-24T14:00:56Z

@bors: r-

This hit a few failures locally in the last rollup (which didn't land):

gnzlbg · 2018-03-24T14:04:40Z

I can fix those tomorrow night earliest at least. But this PR is not time critical. I wonder how come these did not show up earlier :/

…

On Sat 24. Mar 2018 at 15:01, Alex Crichton ***@***.***> wrote: @bors <https://github.com/bors>: r- This hit a few failures locally in the last rollup <#49317> (which didn't land): - missing error-pattern <bbdb20a#diff-433b91cf9a1834bfaa4ac67c9527169a> - bad CHECK patterns <fc23686> — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#49249 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AA3NpvbHefTU8O7uAdP81vj3YAIwvmC9ks5thlG2gaJpZM4S10Kp> .

bors · 2018-03-24T14:04:42Z

@gnzlbg: 🔑 Insufficient privileges: Not in reviewers

bors · 2018-03-26T02:41:56Z

☔ The latest upstream changes (presumably #49297) made this pull request unmergeable. Please resolve the merge conflicts.

gnzlbg · 2018-03-26T08:22:00Z

updated and rebased

alexcrichton · 2018-03-26T08:43:32Z

@bors: r+

bors · 2018-03-26T08:43:33Z

📌 Commit 56aaf34 has been approved by alexcrichton

bors · 2018-03-27T04:46:38Z

⌛ Testing commit 56aaf34 with merge 31ae4f9...

implement minmax intrinsics This adds the `simd_{fmin,fmax}` intrinsics, which do a vertical (lane-wise) `min`/`max` for floating point vectors that's equivalent to Rust's `min`/`max` for `f32`/`f64`. It might make sense to make `{f32,f64}::{min,max}` use the `minnum` and `minmax` intrinsics as well. --- ~~HELP: I need some help with these. Either I should go to sleep or there must be something that I must be missing. AFAICT I am calling the `maxnum` builder correctly, yet rustc/LLVM seem to insert a call to `llvm.minnum` there instead...~~ EDIT: Rust's LLVM version is too old :/

bors · 2018-03-27T07:16:24Z

☀️ Test successful - status-appveyor, status-travis
Approved by: alexcrichton
Pushing 31ae4f9 to master...

rust-highfive assigned nikomatsakis Mar 21, 2018

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Mar 21, 2018

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 22, 2018

bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Mar 22, 2018

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Mar 22, 2018

nikomatsakis assigned alexcrichton and unassigned nikomatsakis Mar 22, 2018

bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Mar 22, 2018

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Mar 22, 2018

alexcrichton mentioned this pull request Mar 23, 2018

Rollup of 20 pull requests #49317

Closed

bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Mar 24, 2018

gnzlbg added 6 commits March 26, 2018 10:20

implement minmax intrinsics

7d5343a

add tests

d2cd57c

require llvm 6

066c2ec

properly handle the case when LLVM does not have min/maxnum

1841f40

set min-llvm-version 6.0, ignore-emscripten

48fd903

fix tests

56aaf34

gnzlbg force-pushed the simd_minmax branch from d87b403 to 56aaf34 Compare March 26, 2018 08:21

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Mar 26, 2018

bors merged commit 56aaf34 into rust-lang:master Mar 27, 2018

gnzlbg mentioned this pull request Apr 6, 2018

simd_fmax is broken in LLVM6 rust-lang/stdarch#416

Closed

gnzlbg mentioned this pull request Jul 19, 2018

simd_fmax is broken in LLVM6 rust-lang/packed_simd#7

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

implement minmax intrinsics #49249

implement minmax intrinsics #49249

gnzlbg commented Mar 21, 2018 •

edited

Loading

rust-highfive commented Mar 21, 2018

gnzlbg commented Mar 21, 2018

gnzlbg commented Mar 21, 2018

alexcrichton commented Mar 21, 2018

gnzlbg commented Mar 21, 2018

gnzlbg commented Mar 22, 2018

alexcrichton commented Mar 22, 2018

bors commented Mar 22, 2018

kennytm commented Mar 22, 2018

kennytm commented Mar 22, 2018

bors commented Mar 22, 2018

kennytm commented Mar 22, 2018

alexcrichton commented Mar 22, 2018

bors commented Mar 22, 2018

alexcrichton commented Mar 24, 2018

gnzlbg commented Mar 24, 2018 via email

bors commented Mar 24, 2018

bors commented Mar 26, 2018

gnzlbg commented Mar 26, 2018

alexcrichton commented Mar 26, 2018

bors commented Mar 26, 2018

bors commented Mar 27, 2018

bors commented Mar 27, 2018

implement minmax intrinsics #49249

implement minmax intrinsics #49249

Conversation

gnzlbg commented Mar 21, 2018 • edited Loading

rust-highfive commented Mar 21, 2018

gnzlbg commented Mar 21, 2018

gnzlbg commented Mar 21, 2018

alexcrichton commented Mar 21, 2018

gnzlbg commented Mar 21, 2018

gnzlbg commented Mar 22, 2018

alexcrichton commented Mar 22, 2018

bors commented Mar 22, 2018

kennytm commented Mar 22, 2018

kennytm commented Mar 22, 2018

bors commented Mar 22, 2018

kennytm commented Mar 22, 2018

alexcrichton commented Mar 22, 2018

bors commented Mar 22, 2018

alexcrichton commented Mar 24, 2018

gnzlbg commented Mar 24, 2018 via email

bors commented Mar 24, 2018

bors commented Mar 26, 2018

gnzlbg commented Mar 26, 2018

alexcrichton commented Mar 26, 2018

bors commented Mar 26, 2018

bors commented Mar 27, 2018

bors commented Mar 27, 2018

gnzlbg commented Mar 21, 2018 •

edited

Loading