Skip to content

x86 (32/64): go back to passing SIMD vectors by-ptr #141309

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

RalfJung
Copy link
Member

@RalfJung RalfJung commented May 20, 2025

Fixes #139029 by partially reverting #135408 and going back to passing SIMD vectors by-ptr on x86. Sadly, by-val confuses the LLVM inliner so much that it's not worth it...

Also fixes #141848 by no longer actually using vector registers with the "Rust" ABI.

r? @tgross35
Cc @nikic

try-job: test-various*
try-job: x86_64-gnu-nopt
try-job: dist-i586-gnu-i586-i686-musl
try-job: x86_64-msvc-1

@rustbot 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. labels May 20, 2025
@RalfJung
Copy link
Member Author

@bors try

bors added a commit to rust-lang-ci/rust that referenced this pull request May 20, 2025
x86 (32/64): go back to passing SIMD vectors by-ptr

Fixes rust-lang#139029 by partially reverting rust-lang#135408 and going back to passing SIMD vectors by-ptr on x86. Sadly, by-val confuses the LLVM inliner so much that it's not worth it...

r? `@tgross35`
Cc `@nikic`

try-job: `test-various*`
try-job: x86_64-gnu-nopt
try-job: dist-i586-gnu-i586-i686-musl
try-job: x86_64-msvc-1
@bors
Copy link
Collaborator

bors commented May 20, 2025

⌛ Trying commit 436f812 with merge 11be405...

Copy link
Contributor

@tgross35 tgross35 left a comment

Choose a reason for hiding this comment

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

The code changes LGTM assuming the try job passes. However, I think it's worth waiting until 1-2 more people weigh in and agree this is worth a revert.

@bors
Copy link
Collaborator

bors commented May 20, 2025

☀️ Try build successful - checks-actions
Build commit: 11be405 (11be4058a43b716323bc6c2f5987d03a309c6d2c)

Copy link
Contributor

@nikic nikic left a comment

Choose a reason for hiding this comment

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

Agree that we should go back to the old ABI for now. Can always switch back after the LLVM issue is fixed.

@RalfJung
Copy link
Member Author

Is there an LLVM issue we can reference?

@RalfJung
Copy link
Member Author

RalfJung commented Jun 1, 2025

However, I think it's worth waiting until 1-2 more people weigh in and agree this is worth a revert.

We got @nikic's +1. Anyone else we should ask?

FWIW this will also help with #141848: for people that disable SSE on an i686 target, the lint at #116558 (hard error on nightly and beta) can actually trigger for "Rust" ABI functions. We don't actually support such configurations, but we did not intent for them to become a hard error so quickly either.

@rust-log-analyzer
Copy link
Collaborator

The job x86_64-gnu-tools failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)
tests/pass-dep/tokio/file-io.rs ... FAILED
tests/pass-dep/libc/mmap.rs ... ok

FAILED TEST: tests/pass-dep/tokio/file-io.rs
command: MIRI_ENV_VAR_TEST="0" MIRI_TEMP="/tmp/miri-uitest-Znr3Ec" RUST_BACKTRACE="1" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/bin/miri" "--error-format=json" "--sysroot=/checkout/obj/build/x86_64-unknown-linux-gnu/miri-sysroot" "-Dwarnings" "-Dunused" "-Ainternal_features" "-Zui-testing" "--out-dir" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage1-tools/x86_64-unknown-linux-gnu/tmp/miri_ui/0/tests/pass-dep/tokio" "tests/pass-dep/tokio/file-io.rs" "-Zmiri-disable-isolation" "--extern" "cfg_if=/checkout/obj/build/x86_64-unknown-linux-gnu/stage1-tools/x86_64-unknown-linux-gnu/tmp/miri_ui/0/miri/x86_64-unknown-linux-gnu/debug/deps/libcfg_if-55a32e410325a882.rlib" "--extern" "cfg_if=/checkout/obj/build/x86_64-unknown-linux-gnu/stage1-tools/x86_64-unknown-linux-gnu/tmp/miri_ui/0/miri/x86_64-unknown-linux-gnu/debug/deps/libcfg_if-55a32e410325a882.rmeta" "--extern" "getrandom_01=/checkout/obj/build/x86_64-unknown-linux-gnu/stage1-tools/x86_64-unknown-linux-gnu/tmp/miri_ui/0/miri/x86_64-unknown-linux-gnu/debug/deps/libgetrandom-77190f9b44a3c009.rlib" "--extern" "getrandom_01=/checkout/obj/build/x86_64-unknown-linux-gnu/stage1-tools/x86_64-unknown-linux-gnu/tmp/miri_ui/0/miri/x86_64-unknown-linux-gnu/debug/deps/libgetrandom-77190f9b44a3c009.rmeta" "--extern" "getrandom_02=/checkout/obj/build/x86_64-unknown-linux-gnu/stage1-tools/x86_64-unknown-linux-gnu/tmp/miri_ui/0/miri/x86_64-unknown-linux-gnu/debug/deps/libgetrandom-56888fca95a09bf8.rlib" "--extern" "getrandom_02=/checkout/obj/build/x86_64-unknown-linux-gnu/stage1-tools/x86_64-unknown-linux-gnu/tmp/miri_ui/0/miri/x86_64-unknown-linux-gnu/debug/deps/libgetrandom-56888fca95a09bf8.rmeta" "--extern" "getrandom_03=/checkout/obj/build/x86_64-unknown-linux-gnu/stage1-tools/x86_64-unknown-linux-gnu/tmp/miri_ui/0/miri/x86_64-unknown-linux-gnu/debug/deps/libgetrandom-a8fc577bb44b6d80.rlib" "--extern" "getrandom_03=/checkout/obj/build/x86_64-unknown-linux-gnu/stage1-tools/x86_64-unknown-linux-gnu/tmp/miri_ui/0/miri/x86_64-unknown-linux-gnu/debug/deps/libgetrandom-a8fc577bb44b6d80.rmeta" "--extern" "libc=/checkout/obj/build/x86_64-unknown-linux-gnu/stage1-tools/x86_64-unknown-linux-gnu/tmp/miri_ui/0/miri/x86_64-unknown-linux-gnu/debug/deps/liblibc-dd98cac8117d550a.rlib" "--extern" "libc=/checkout/obj/build/x86_64-unknown-linux-gnu/stage1-tools/x86_64-unknown-linux-gnu/tmp/miri_ui/0/miri/x86_64-unknown-linux-gnu/debug/deps/liblibc-dd98cac8117d550a.rmeta" "--extern" "num_cpus=/checkout/obj/build/x86_64-unknown-linux-gnu/stage1-tools/x86_64-unknown-linux-gnu/tmp/miri_ui/0/miri/x86_64-unknown-linux-gnu/debug/deps/libnum_cpus-7ab8becb6da559b7.rlib" "--extern" "num_cpus=/checkout/obj/build/x86_64-unknown-linux-gnu/stage1-tools/x86_64-unknown-linux-gnu/tmp/miri_ui/0/miri/x86_64-unknown-linux-gnu/debug/deps/libnum_cpus-7ab8becb6da559b7.rmeta" "--extern" "page_size=/checkout/obj/build/x86_64-unknown-linux-gnu/stage1-tools/x86_64-unknown-linux-gnu/tmp/miri_ui/0/miri/x86_64-unknown-linux-gnu/debug/deps/libpage_size-c34469ad90c5eb76.rlib" "--extern" "page_size=/checkout/obj/build/x86_64-unknown-linux-gnu/stage1-tools/x86_64-unknown-linux-gnu/tmp/miri_ui/0/miri/x86_64-unknown-linux-gnu/debug/deps/libpage_size-c34469ad90c5eb76.rmeta" "--extern" "tempfile=/checkout/obj/build/x86_64-unknown-linux-gnu/stage1-tools/x86_64-unknown-linux-gnu/tmp/miri_ui/0/miri/x86_64-unknown-linux-gnu/debug/deps/libtempfile-d40c54a83e8abae9.rlib" "--extern" "tempfile=/checkout/obj/build/x86_64-unknown-linux-gnu/stage1-tools/x86_64-unknown-linux-gnu/tmp/miri_ui/0/miri/x86_64-unknown-linux-gnu/debug/deps/libtempfile-d40c54a83e8abae9.rmeta" "--extern" "tokio=/checkout/obj/build/x86_64-unknown-linux-gnu/stage1-tools/x86_64-unknown-linux-gnu/tmp/miri_ui/0/miri/x86_64-unknown-linux-gnu/debug/deps/libtokio-539f2d149eae570a.rlib" "--extern" "tokio=/checkout/obj/build/x86_64-unknown-linux-gnu/stage1-tools/x86_64-unknown-linux-gnu/tmp/miri_ui/0/miri/x86_64-unknown-linux-gnu/debug/deps/libtokio-539f2d149eae570a.rmeta" "-L" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage1-tools/x86_64-unknown-linux-gnu/tmp/miri_ui/0/miri/x86_64-unknown-linux-gnu/debug/deps" "-L" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage1-tools/x86_64-unknown-linux-gnu/tmp/miri_ui/0/miri/debug/deps" "--edition" "2021"

error: test got exit status: 101, but expected 0
 = note: the compiler panicked

error: no output was expected
---
   4: test_create_and_write::{closure#0}
             at tests/pass-dep/tokio/file-io.rs:24:5
   5: main::{closure#0}
             at tests/pass-dep/tokio/file-io.rs:14:29
   6: tokio::runtime::park::CachedParkThread::block_on::<{async block@tests/pass-dep/tokio/file-io.rs:12:1: 12:15}>::{closure#0}
             at /cargo/registry/src/index.crates.io-1949cf8c6b5b557f/tokio-1.44.2/src/runtime/park.rs:284:60
   7: tokio::task::coop::with_budget::<std::task::Poll<()>, {closure@tokio::runtime::park::CachedParkThread::block_on<{async block@tests/pass-dep/tokio/file-io.rs:12:1: 12:15}>::{closure#0}}>
             at /cargo/registry/src/index.crates.io-1949cf8c6b5b557f/tokio-1.44.2/src/task/coop/mod.rs:167:5
   8: tokio::task::coop::budget::<std::task::Poll<()>, {closure@tokio::runtime::park::CachedParkThread::block_on<{async block@tests/pass-dep/tokio/file-io.rs:12:1: 12:15}>::{closure#0}}>
             at /cargo/registry/src/index.crates.io-1949cf8c6b5b557f/tokio-1.44.2/src/task/coop/mod.rs:133:5
   9: tokio::runtime::park::CachedParkThread::block_on::<{async block@tests/pass-dep/tokio/file-io.rs:12:1: 12:15}>
             at /cargo/registry/src/index.crates.io-1949cf8c6b5b557f/tokio-1.44.2/src/runtime/park.rs:284:31
  10: tokio::runtime::context::blocking::BlockingRegionGuard::block_on::<{async block@tests/pass-dep/tokio/file-io.rs:12:1: 12:15}>
             at /cargo/registry/src/index.crates.io-1949cf8c6b5b557f/tokio-1.44.2/src/runtime/context/blocking.rs:66:9
  11: tokio::runtime::scheduler::multi_thread::MultiThread::block_on::<{async block@tests/pass-dep/tokio/file-io.rs:12:1: 12:15}>::{closure#0}
             at /cargo/registry/src/index.crates.io-1949cf8c6b5b557f/tokio-1.44.2/src/runtime/scheduler/multi_thread/mod.rs:87:13
  12: tokio::runtime::context::runtime::enter_runtime::<{closure@tokio::runtime::scheduler::multi_thread::MultiThread::block_on<{async block@tests/pass-dep/tokio/file-io.rs:12:1: 12:15}>::{closure#0}}, ()>
             at /cargo/registry/src/index.crates.io-1949cf8c6b5b557f/tokio-1.44.2/src/runtime/context/runtime.rs:65:16
  13: tokio::runtime::scheduler::multi_thread::MultiThread::block_on::<{async block@tests/pass-dep/tokio/file-io.rs:12:1: 12:15}>
             at /cargo/registry/src/index.crates.io-1949cf8c6b5b557f/tokio-1.44.2/src/runtime/scheduler/multi_thread/mod.rs:86:9
  14: tokio::runtime::Runtime::block_on_inner::<{async block@tests/pass-dep/tokio/file-io.rs:12:1: 12:15}>
             at /cargo/registry/src/index.crates.io-1949cf8c6b5b557f/tokio-1.44.2/src/runtime/runtime.rs:370:45
  15: tokio::runtime::Runtime::block_on::<{async block@tests/pass-dep/tokio/file-io.rs:12:1: 12:15}>
             at /cargo/registry/src/index.crates.io-1949cf8c6b5b557f/tokio-1.44.2/src/runtime/runtime.rs:342:13
  16: main
             at tests/pass-dep/tokio/file-io.rs:15:5
  17: <fn() as std::ops::FnOnce<()>>::call_once - shim(fn())
             at /checkout/library/core/src/ops/function.rs:250:5
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.
---
   4: test_create_and_write::{closure#0}
             at tests/pass-dep/tokio/file-io.rs:24:5
   5: main::{closure#0}
             at tests/pass-dep/tokio/file-io.rs:14:29
   6: tokio::runtime::park::CachedParkThread::block_on::<{async block@tests/pass-dep/tokio/file-io.rs:12:1: 12:15}>::{closure#0}
             at /cargo/registry/src/index.crates.io-1949cf8c6b5b557f/tokio-1.44.2/src/runtime/park.rs:284:60
   7: tokio::task::coop::with_budget::<std::task::Poll<()>, {closure@tokio::runtime::park::CachedParkThread::block_on<{async block@tests/pass-dep/tokio/file-io.rs:12:1: 12:15}>::{closure#0}}>
             at /cargo/registry/src/index.crates.io-1949cf8c6b5b557f/tokio-1.44.2/src/task/coop/mod.rs:167:5
   8: tokio::task::coop::budget::<std::task::Poll<()>, {closure@tokio::runtime::park::CachedParkThread::block_on<{async block@tests/pass-dep/tokio/file-io.rs:12:1: 12:15}>::{closure#0}}>
             at /cargo/registry/src/index.crates.io-1949cf8c6b5b557f/tokio-1.44.2/src/task/coop/mod.rs:133:5
   9: tokio::runtime::park::CachedParkThread::block_on::<{async block@tests/pass-dep/tokio/file-io.rs:12:1: 12:15}>
             at /cargo/registry/src/index.crates.io-1949cf8c6b5b557f/tokio-1.44.2/src/runtime/park.rs:284:31
  10: tokio::runtime::context::blocking::BlockingRegionGuard::block_on::<{async block@tests/pass-dep/tokio/file-io.rs:12:1: 12:15}>
             at /cargo/registry/src/index.crates.io-1949cf8c6b5b557f/tokio-1.44.2/src/runtime/context/blocking.rs:66:9
  11: tokio::runtime::scheduler::multi_thread::MultiThread::block_on::<{async block@tests/pass-dep/tokio/file-io.rs:12:1: 12:15}>::{closure#0}
             at /cargo/registry/src/index.crates.io-1949cf8c6b5b557f/tokio-1.44.2/src/runtime/scheduler/multi_thread/mod.rs:87:13
  12: tokio::runtime::context::runtime::enter_runtime::<{closure@tokio::runtime::scheduler::multi_thread::MultiThread::block_on<{async block@tests/pass-dep/tokio/file-io.rs:12:1: 12:15}>::{closure#0}}, ()>
             at /cargo/registry/src/index.crates.io-1949cf8c6b5b557f/tokio-1.44.2/src/runtime/context/runtime.rs:65:16
  13: tokio::runtime::scheduler::multi_thread::MultiThread::block_on::<{async block@tests/pass-dep/tokio/file-io.rs:12:1: 12:15}>
             at /cargo/registry/src/index.crates.io-1949cf8c6b5b557f/tokio-1.44.2/src/runtime/scheduler/multi_thread/mod.rs:86:9
  14: tokio::runtime::Runtime::block_on_inner::<{async block@tests/pass-dep/tokio/file-io.rs:12:1: 12:15}>
             at /cargo/registry/src/index.crates.io-1949cf8c6b5b557f/tokio-1.44.2/src/runtime/runtime.rs:370:45
  15: tokio::runtime::Runtime::block_on::<{async block@tests/pass-dep/tokio/file-io.rs:12:1: 12:15}>
             at /cargo/registry/src/index.crates.io-1949cf8c6b5b557f/tokio-1.44.2/src/runtime/runtime.rs:342:13
  16: main
             at tests/pass-dep/tokio/file-io.rs:15:5
  17: <fn() as std::ops::FnOnce<()>>::call_once - shim(fn())
             at /checkout/library/core/src/ops/function.rs:250:5
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.
---

Location:
   /cargo/registry/src/index.crates.io-1949cf8c6b5b557f/ui_test-0.29.2/src/lib.rs:369

Backtrace omitted. Run with RUST_BACKTRACE=1 environment variable to display it.
Run with RUST_BACKTRACE=full to include source snippets.
error: test failed, to rerun pass `--test ui`

Caused by:
  process didn't exit successfully: `/checkout/obj/build/x86_64-unknown-linux-gnu/stage1-tools/x86_64-unknown-linux-gnu/release/deps/ui-f58affc77002becb` (exit status: 1)
Command has failed. Rerun with -v to see more details.
Build completed unsuccessfully in 0:05:04
  local time: Sun Jun  1 09:23:35 UTC 2025
  network time: Sun, 01 Jun 2025 09:23:35 GMT
##[error]Process completed with exit code 1.
Post job cleanup.

@RalfJung
Copy link
Member Author

RalfJung commented Jun 1, 2025

(That's rust-lang/miri#4367)

@RalfJung
Copy link
Member Author

RalfJung commented Jun 1, 2025

We may even consider backporting this, to avoid Rust 1.88 being the only release where a "Rust" ABI function with a by-value SIMD vector on an i686 target modified to disable SSE causes a hard error. (That's not a configuration we intend to support, but it wasn't really meant to hard-error quite so quickly.) See #141848 for context.

@RalfJung RalfJung added the beta-nominated Nominated for backporting to the compiler in the beta channel. label Jun 1, 2025
@tgross35
Copy link
Contributor

tgross35 commented Jun 1, 2025

We got @nikic's +1. Anyone else we should ask?

I think it's fine to merge with the CI fix 👍 a few other people have looked at this and upvoted the backport, so seems like there's more consensus there anyway.

@RalfJung
Copy link
Member Author

RalfJung commented Jun 1, 2025 via email

@tgross35
Copy link
Contributor

tgross35 commented Jun 1, 2025

Oops I missed that the linked Miri issue has since been fixed.

@tgross35
Copy link
Contributor

tgross35 commented Jun 1, 2025

(mistyped the bors command)

@bors r=tgross35,nikic

@bors
Copy link
Collaborator

bors commented Jun 1, 2025

📌 Commit a4d314f has been approved by tgross35,nikic

It is now in the queue for this repository.

@bors 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 Jun 1, 2025
@workingjubilee
Copy link
Member

lgtm

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Jun 2, 2025
…nikic

x86 (32/64): go back to passing SIMD vectors by-ptr

Fixes rust-lang#139029 by partially reverting rust-lang#135408 and going back to passing SIMD vectors by-ptr on x86. Sadly, by-val confuses the LLVM inliner so much that it's not worth it...

Also fixes rust-lang#141848 by no longer actually using vector registers with the "Rust" ABI.

r? `@tgross35`
Cc `@nikic`

try-job: `test-various*`
try-job: x86_64-gnu-nopt
try-job: dist-i586-gnu-i586-i686-musl
try-job: x86_64-msvc-1
bors added a commit that referenced this pull request Jun 2, 2025
Rollup of 6 pull requests

Successful merges:

 - #140715 (Clarify &mut-methods' docs on sync::OnceLock)
 - #141309 (x86 (32/64): go back to passing SIMD vectors by-ptr)
 - #141677 (Async drop - type instead of async drop fn, fixes #140484)
 - #141733 (C-variadic functions must be unsafe)
 - #141858 (Fix typo in `StructuralPartialEq` docs)
 - #141874 (add f16_epsilon and f128_epsilon diagnostic items)

r? `@ghost`
`@rustbot` modify labels: rollup
@matthiaskrgr
Copy link
Member

@bors r-

#141879 (comment)

@bors 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 Jun 2, 2025
@RalfJung
Copy link
Member Author

RalfJung commented Jun 2, 2025 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
beta-nominated Nominated for backporting to the compiler in the beta channel. S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
8 participants