Bump stdarch submodule and remove special handling for LLVM intrinsics that are no longer needed by eduardosm · Pull Request #116609 · rust-lang/rust

eduardosm · 2023-10-10T17:22:29Z

Bumps stdarch to pull rust-lang/stdarch#1477, which reimplemented some functions with portable SIMD intrinsics instead of arch specific LLVM intrinsics.

Handling of those LLVM intrinsics is removed from cranelift codegen and miri.

cc @RalfJung @bjorn3

rustbot · 2023-10-10T17:22:37Z

r? @b-naber

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

rustbot · 2023-10-10T17:22:39Z

⚠️Warning⚠️

These commits modify submodules.

rustbot · 2023-10-10T17:23:11Z

The Miri subtree was changed

cc @rust-lang/miri

Some changes occurred in compiler/rustc_codegen_cranelift

cc @bjorn3

RalfJung · 2023-10-11T05:49:18Z

- assert_eq_m128i(r, e);
- }
- test_mm_mul_epu32();
-


Why remove the tests? They should still pass, right? I think we should keep them, even if they now just provide extra test coverage for the simd intrinsics. They also help ensure that we actually do support the ABI surface of core::arch.

I can keep them if you want, but those wouldn't have landed in Miri in the first place if the functions hadn't used LLVM intrinsics from the beginning. Doesn't miri-test-libstd give that extra coverage?

We like to have basic coverage in the Miri test suite since that runs in rustc CI.
So yeah please keep the tests. Ideally we'd invoke every public stable function at least once to make sure that codepath doesn't hit "unsupported" errors, but with the arch intrinsics that might be asking too much... but when we do have a test, there's no reason to remove it.

eduardosm · 2023-10-11T19:09:42Z

So simd_bitmask has stricter input requirements than the equivalent SSE instructions, I will have to fix that in stdarch.
Good thing you decided to keep the tests @RalfJung :)

RalfJung · 2023-10-11T19:17:42Z

Ah yeah we've run into that one before.^^ That was rust-lang/miri#2617.

eduardosm · 2023-10-11T20:11:40Z

I will update this PR once rust-lang/stdarch#1482 is merged. This is one of the moments were I would have liked stdarch to be a sub-tree instead of a submodule.

RalfJung · 2023-10-11T20:17:40Z

Cc @workingjubilee this is the second time simd_bitmask turned out to be a footgun

workingjubilee · 2023-10-11T20:19:22Z

eh? can you remind me, what was the first?

RalfJung · 2023-10-11T20:22:26Z

rust-lang/stdarch#1347

eduardosm · 2023-10-12T10:22:09Z

Fixed :)

b-naber · 2023-10-23T20:20:19Z

r? compiler

workingjubilee · 2023-10-28T07:18:59Z

Thanks!
r? @workingjubilee
@bors r+

bors · 2023-10-28T07:19:01Z

📌 Commit 35e2f4e has been approved by workingjubilee

It is now in the queue for this repository.

…ubilee Bump stdarch submodule and remove special handling for LLVM intrinsics that are no longer needed Bumps stdarch to pull rust-lang/stdarch#1477, which reimplemented some functions with portable SIMD intrinsics instead of arch specific LLVM intrinsics. Handling of those LLVM intrinsics is removed from cranelift codegen and miri. cc `@RalfJung` `@bjorn3`

…iaskrgr Rollup of 8 pull requests Successful merges: - rust-lang#116534 (Remove -Zdep-tasks.) - rust-lang#116609 (Bump stdarch submodule and remove special handling for LLVM intrinsics that are no longer needed) - rust-lang#116739 (Make `E0277` use short paths) - rust-lang#116945 (When encountering sealed traits, point types that implement it) - rust-lang#117025 (Cleanup and improve `--check-cfg` implementation) - rust-lang#117029 (Add FileCheck annotations to MIR-opt inlining tests ) - rust-lang#117256 (Parse rustc version at compile time) - rust-lang#117277 (fix failure to detect a too-big-type after adding padding) r? `@ghost` `@rustbot` modify labels: rollup

workingjubilee · 2023-10-28T08:08:02Z

@bors rollup=never

bors · 2023-10-28T11:26:38Z

⌛ Testing commit 35e2f4e with merge 3089c31...

bors · 2023-10-28T13:19:53Z

☀️ Test successful - checks-actions
Approved by: workingjubilee
Pushing 3089c31 to master...

rust-timer · 2023-10-28T15:08:05Z

Finished benchmarking commit (3089c31): comparison URL.

Overall result: no relevant changes - no action needed

@rustbot label: -perf-regression

Instruction count

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

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)	-	-	0
Improvements ✅ (secondary)	-3.2%	[-3.4%, -2.9%]	2
All ❌✅ (primary)	-	-	0

Cycles

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

Binary size

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

Bootstrap: 635.895s -> 637.216s (0.21%)
Artifact size: 304.43 MiB -> 304.42 MiB (-0.00%)

…ilee Bump stdarch submodule and remove special handling for LLVM intrinsics that are no longer needed Bumps stdarch to pull rust-lang/stdarch#1477, which reimplemented some functions with portable SIMD intrinsics instead of arch specific LLVM intrinsics. Handling of those LLVM intrinsics is removed from cranelift codegen and miri. cc `@RalfJung` `@bjorn3`

rustbot assigned b-naber Oct 10, 2023

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 Oct 10, 2023

RalfJung reviewed Oct 11, 2023
View reviewed changes

This comment has been minimized.
Sign in to view

eduardosm added 4 commits October 12, 2023 11:11

Bump stdarch submodule
89f9da2

Remove from cranelift codegen LLVM intrinsics that are no longer needed
b57a157

Remove from miri LLVM intrinsics that are no longer needed
337af7c

Fix identation of a rustfmt::skiped statement
35e2f4e

rustbot assigned cjgillot and unassigned b-naber Oct 23, 2023

rustbot assigned workingjubilee and unassigned cjgillot Oct 28, 2023

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 Oct 28, 2023

matthiaskrgr mentioned this pull request Oct 28, 2023
Rollup of 8 pull requests #117308
Closed

bors added the merged-by-bors This PR was explicitly merged by bors. label Oct 28, 2023

bors merged commit 3089c31 into rust-lang:masterOct 28, 2023

rustbot added this to the 1.75.0 milestone Oct 28, 2023

eduardosm deleted the bump-stdarch branch October 28, 2023 17:04

github-actionsBot mentioned this pull request Oct 31, 2023
Automated pull from upstream masterferrocene/ferrocene#74
Merged

moxian mentioned this pull request Mar 20, 2025
Performance regression: rustc failed to optimize specific x86-64 SIMD intrinsics after 1.75.0 #124216
Open

Uh oh!

Uh oh!

Conversation

eduardosm commented Oct 10, 2023

Uh oh!

rustbot commented Oct 10, 2023

Uh oh!

rustbot commented Oct 10, 2023

Uh oh!

rustbot commented Oct 10, 2023

Uh oh!

RalfJungOct 11, 2023

Choose a reason for hiding this comment

Uh oh!

eduardosmOct 11, 2023

Choose a reason for hiding this comment

Uh oh!

RalfJungOct 11, 2023

Choose a reason for hiding this comment

Uh oh!

This comment has been minimized.

eduardosm commented Oct 11, 2023

Uh oh!

RalfJung commented Oct 11, 2023

Uh oh!

eduardosm commented Oct 11, 2023

Uh oh!

RalfJung commented Oct 11, 2023

Uh oh!

workingjubilee commented Oct 11, 2023

Uh oh!

RalfJung commented Oct 11, 2023

Uh oh!

eduardosm commented Oct 12, 2023

Uh oh!

b-naber commented Oct 23, 2023

Uh oh!

workingjubilee commented Oct 28, 2023

Uh oh!

bors commented Oct 28, 2023

Uh oh!

workingjubilee commented Oct 28, 2023

Uh oh!

bors commented Oct 28, 2023

Uh oh!

bors commented Oct 28, 2023

Uh oh!

rust-timer commented Oct 28, 2023

Overall result: no relevant changes - no action needed

Instruction count

Max RSS (memory usage)

Cycles

Binary size

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants