remove platform-intrinsics ABI; make SIMD intrinsics be regular intrinsics #121516

RalfJung · 2024-02-23T17:28:09Z

@Amanieu @workingjubilee I don't think there is any reason these need to be "special"? The original RFC indicated eventually making them stable, but I think that is no longer the plan, so seems to me like we can clean this up a bit.

Blocked on rust-lang/stdarch#1538, #121542.

rustbot · 2024-02-23T17:28:17Z

r? @Mark-Simulacrum

rustbot has assigned @Mark-Simulacrum.
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

rustbot · 2024-02-23T17:28:19Z

Some changes occurred to MIR optimizations

cc @rust-lang/wg-mir-opt

This PR changes Stable MIR

cc @oli-obk, @celinval, @spastorino, @ouz-a

Some changes occurred to the platform-builtins intrinsics. Make sure the
LLVM backend as well as portable-simd gets adapted for the changes.

cc @antoyo, @GuillaumeGomez, @bjorn3, @calebzulawski, @programmerjake

Some changes occurred in compiler/rustc_codegen_cranelift

cc @bjorn3

These commits modify compiler targets.
(See the Target Tier Policy.)

compiler/rustc_middle/src/ty/layout.rs

RalfJung · 2024-02-23T17:31:42Z

compiler/rustc_ty_utils/src/abi.rs

                // that's how we connect up to LLVM and it's unstable
                // anyway, we control all calls to it in libstd.
                Abi::Vector { .. }
-                    if abi != SpecAbi::PlatformIntrinsic
-                        && cx.tcx.sess.target.simd_types_indirect =>
+                    if abi != SpecAbi::RustIntrinsic && cx.tcx.sess.target.simd_types_indirect =>


This means we're no longer applying the make-simd-indirect ABI adjustment to our non-SIMD intrinsics. But those aren't actual function calls anyway so I think that's fine?

rustbot · 2024-02-23T17:37:18Z

The Miri subtree was changed

cc @rust-lang/miri

library/core/src/intrinsics/simd.rs

compiler/rustc_codegen_cranelift/example/float-minmax-pass.rs

workingjubilee · 2024-02-23T18:55:05Z

error[E0703]: invalid ABI: found intrinsic
--> library/core/src/intrinsics/simd.rs:594:8
|
|
594 | extern "intrinsic" {
| ^^^^^^^^^^^ invalid ABI

error[E0044]: foreign items may not have type parameters

well, these seem like non-starters.
did you mean extern "rust-intrinsic"?

RalfJung · 2024-02-23T18:58:31Z

did you mean extern "rust-intrinsic"?

🤦

rustbot · 2024-02-23T19:00:15Z

This PR changes MIR

cc @oli-obk, @RalfJung, @JakobDegen, @davidtwco, @celinval, @vakaras

workingjubilee · 2024-02-23T19:08:41Z

This seems fine to me. There was indeed once a plan to stabilize these as a surface interface, but it was during a point in time when it was thought that e.g. add-with-carry would require these. The overwhelming tendency has been against that over time, and making things at least surface as more "normal" library abstractions. SIMD intrinsics do have a distinctive "expected UX" (and by "UX", I mean "they should codegen a certain way"), but so do most rust-intrinsics, which generally serve some codegen purpose and each require at least some special handling.

In each case we may address these needs by handling these symbols individually, or by adding an attribute which covers their need.

Amanieu · 2024-02-24T00:08:39Z

LGTM. It's clear the design for SIMD has moved away from these intrinsics towards std::{arch,simd} instead.

oli-obk · 2024-02-24T00:18:14Z

tests/ui/simd/intrinsic/float-minmax-pass.rs

-extern "platform-intrinsic" {
-    fn simd_fmin<T>(x: T, y: T) -> T;
-    fn simd_fmax<T>(x: T, y: T) -> T;
-}
+use std::intrinsics::simd::*;


I'm assuming the reason only some of these got converted to imports in tests is that it's annoying to do with a search and replace?

Yeah, I changed this one since @bjorn3 mentioned it but most of the diff in tests/ui is search-and-replace. it'd be tedious to convert them all and it's orthogonal to what this PR is trying to achieve.

Oh yea, that's fine, just wanted to make sure I wasn't missing anything

oli-obk · 2024-02-24T19:10:07Z

@bors r-

I guess we're waiting for the submodule sync PR first

r=me with that done

bors · 2024-02-24T23:56:04Z

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

…nsics

RalfJung · 2024-02-25T07:22:01Z

@bors r=oli-obk

bors · 2024-02-25T07:22:03Z

📌 Commit c1d0e48 has been approved by oli-obk

It is now in the queue for this repository.

bors · 2024-02-26T22:24:19Z

⌛ Testing commit c1d0e48 with merge 5c786a7...

bors · 2024-02-27T00:54:07Z

☀️ Test successful - checks-actions
Approved by: oli-obk
Pushing 5c786a7 to master...

rust-timer · 2024-02-27T02:08:18Z

Finished benchmarking commit (5c786a7): comparison URL.

Overall result: ✅ improvements - no action needed

@rustbot label: -perf-regression

Instruction count

This is a highly reliable metric that was used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-1.2%	[-1.2%, -1.2%]	1
All ❌✅ (primary)	-	-	0

Max RSS (memory usage)

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-4.2%	[-4.2%, -4.2%]	1
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-4.2%	[-4.2%, -4.2%]	1

Cycles

This benchmark run did not return any relevant results for this metric.

Binary size

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-0.0%	[-0.0%, -0.0%]	4
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-0.0%	[-0.0%, -0.0%]	4

Bootstrap: 651.881s -> 650.825s (-0.16%)
Artifact size: 311.16 MiB -> 311.12 MiB (-0.01%)

Upgrades the Rust toolchain to `nightly-2024-03-01`. The Rust compiler PRs that triggered changes in this upgrades are: * rust-lang/rust#121516 * rust-lang/rust#121598 * rust-lang/rust#121489 * rust-lang/rust#121783

… r=oli-obk remove platform-intrinsics ABI; make SIMD intrinsics be regular intrinsics `@Amanieu` `@workingjubilee` I don't think there is any reason these need to be "special"? The [original RFC](https://rust-lang.github.io/rfcs/1199-simd-infrastructure.html) indicated eventually making them stable, but I think that is no longer the plan, so seems to me like we can clean this up a bit. Blocked on rust-lang/stdarch#1538, rust-lang#121542.

rustbot assigned Mark-Simulacrum Feb 23, 2024

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Feb 23, 2024

RalfJung commented Feb 23, 2024

View reviewed changes

compiler/rustc_middle/src/ty/layout.rs Show resolved Hide resolved

RalfJung commented Feb 23, 2024

View reviewed changes

This comment has been minimized.

Sign in to view

bjorn3 reviewed Feb 23, 2024

View reviewed changes

library/core/src/intrinsics/simd.rs Outdated Show resolved Hide resolved

bjorn3 reviewed Feb 23, 2024

View reviewed changes

compiler/rustc_codegen_cranelift/example/float-minmax-pass.rs Outdated Show resolved Hide resolved

RalfJung force-pushed the platform-intrinsics-begone branch from 794721e to 774c8f9 Compare February 23, 2024 18:40

This comment has been minimized.

Sign in to view

oli-obk assigned oli-obk and unassigned Mark-Simulacrum Feb 23, 2024

RalfJung force-pushed the platform-intrinsics-begone branch from 774c8f9 to 30f7d92 Compare February 23, 2024 19:00

RalfJung force-pushed the platform-intrinsics-begone branch from 30f7d92 to 188c583 Compare February 23, 2024 19:01

This comment has been minimized.

Sign in to view

oli-obk reviewed Feb 24, 2024

View reviewed changes

RalfJung force-pushed the platform-intrinsics-begone branch 2 times, most recently from 7415e4c to 65d0dbe Compare February 24, 2024 12:10

This comment has been minimized.

Sign in to view

RalfJung force-pushed the platform-intrinsics-begone branch from 65d0dbe to f0b4f4c Compare February 24, 2024 13:09

bors added the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Feb 24, 2024

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 Feb 24, 2024

RalfJung added S-blocked Status: Marked as blocked ❌ on something else such as an RFC or other implementation work. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Feb 24, 2024

RalfJung added 2 commits February 25, 2024 08:14

remove platform-intrinsics ABI; make SIMD intrinsics be regular intri…

cc3df0a

…nsics

fix use of platform_intrinsics in tests

c1d0e48

RalfJung force-pushed the platform-intrinsics-begone branch from 22904b8 to c1d0e48 Compare February 25, 2024 07:15

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-blocked Status: Marked as blocked ❌ on something else such as an RFC or other implementation work. labels Feb 25, 2024

bors added the merged-by-bors This PR was explicitly merged by bors. label Feb 27, 2024

bors merged commit 5c786a7 into rust-lang:master Feb 27, 2024
12 checks passed

rustbot added this to the 1.78.0 milestone Feb 27, 2024

RalfJung deleted the platform-intrinsics-begone branch February 28, 2024 06:30

adpaco-aws mentioned this pull request Mar 1, 2024

Upgrade Rust toolchain to nightly-2024-03-01 model-checking/kani#3052

Merged

GrigorenkoPV mentioned this pull request Jun 17, 2024

Allow reifying intrinsics to fn pointers. (rebase of #86699) #126595

Open

workingjubilee mentioned this pull request Jun 25, 2024

Tracking issues for unstable language features used by std #94970

Open

47 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

remove platform-intrinsics ABI; make SIMD intrinsics be regular intrinsics #121516

remove platform-intrinsics ABI; make SIMD intrinsics be regular intrinsics #121516

RalfJung commented Feb 23, 2024 •

edited

Loading

rustbot commented Feb 23, 2024

rustbot commented Feb 23, 2024

RalfJung Feb 23, 2024

This comment has been minimized.

rustbot commented Feb 23, 2024

This comment has been minimized.

This comment has been minimized.

workingjubilee commented Feb 23, 2024

RalfJung commented Feb 23, 2024

rustbot commented Feb 23, 2024

This comment has been minimized.

workingjubilee commented Feb 23, 2024 •

edited

Loading

Amanieu commented Feb 24, 2024

oli-obk Feb 24, 2024

RalfJung Feb 24, 2024

oli-obk Feb 24, 2024

This comment has been minimized.

oli-obk commented Feb 24, 2024

bors commented Feb 24, 2024

RalfJung commented Feb 25, 2024

bors commented Feb 25, 2024

bors commented Feb 26, 2024

bors commented Feb 27, 2024

rust-timer commented Feb 27, 2024

remove platform-intrinsics ABI; make SIMD intrinsics be regular intrinsics #121516

remove platform-intrinsics ABI; make SIMD intrinsics be regular intrinsics #121516

Conversation

RalfJung commented Feb 23, 2024 • edited Loading

rustbot commented Feb 23, 2024

rustbot commented Feb 23, 2024

Choose a reason for hiding this comment

This comment has been minimized.

rustbot commented Feb 23, 2024

This comment has been minimized.

This comment has been minimized.

workingjubilee commented Feb 23, 2024

RalfJung commented Feb 23, 2024

rustbot commented Feb 23, 2024

This comment has been minimized.

workingjubilee commented Feb 23, 2024 • edited Loading

Amanieu commented Feb 24, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

This comment has been minimized.

oli-obk commented Feb 24, 2024

bors commented Feb 24, 2024

RalfJung commented Feb 25, 2024

bors commented Feb 25, 2024

bors commented Feb 26, 2024

bors commented Feb 27, 2024

rust-timer commented Feb 27, 2024

Overall result: ✅ improvements - no action needed

RalfJung commented Feb 23, 2024 •

edited

Loading

workingjubilee commented Feb 23, 2024 •

edited

Loading