Let codegen decide when to `mem::swap` with immediates by scottmcm · Pull Request #122582 · rust-lang/rust

scottmcm · 2024-03-16T05:14:09Z

Making libcore decide this is silly; the backend has so much better information about when it's a good idea.

Thus this PR introduces a new typed_swap intrinsic with a fallback body, and replaces that fallback implementation when swapping immediates or scalar pairs.

r? oli-obk

Replaces #111744, and means we'll never need more libs PRs like #111803 or #107140

src/tools/miri/tests/fail/intrinsics/typed-swap-invalid-array.rs

rustbot · 2024-03-16T09:23:10Z

Some changes occurred to the CTFE / Miri engine

cc @rust-lang/miri

The Miri subtree was changed

cc @rust-lang/miri

library/core/src/mem/mod.rs

scottmcm · 2024-03-16T19:07:29Z

@bors try @rust-timer queue

Let codegen decide when to `mem::swap` with immediates Making `libcore` decide this is silly; the backend has so much better information about when it's a good idea. Thus this PR introduces a new `typed_swap` intrinsic with a fallback body, and replaces that fallback implementation when swapping immediates or scalar pairs. r? oli-obk Replaces rust-lang#111744, and means we'll never need more libs PRs like rust-lang#111803 or rust-lang#107140

bors · 2024-03-16T19:08:39Z

⌛ Trying commit e583c57 with merge 3f68c63...

bors · 2024-03-16T20:40:43Z

☀️ Try build successful - checks-actions
Build commit: 3f68c63 (3f68c63f114f4d57b01ae74bc549e4b68f955eee)

library/core/src/ptr/mod.rs

RalfJung · 2024-03-17T08:41:22Z

library/core/src/ptr/mod.rs

    }
 }

+/// Non-overlapping *typed* swap of a single value.


Should it be called typed_swap_nonoverlapping?

I went back and forth multiple times on this. typed_swap_nonoverlapping sounds to me like ptr::swap_nonoverlapping, just typed. So that would make this typed_swap_nonoverlapping_one or something, and now that's just seeming really long.

So since it's internal, I think I'm inclined to leave it as-is for now unless someone feels particularly strongly about it.

I feel like the "nonoverlapping" distinction is way more fundamental than the "one" distinction. And since it is internal, consistency with ptr::swap_nonoverlapping isn't very important IMO.

library/core/src/mem/mod.rs

compiler/rustc_const_eval/src/interpret/intrinsics.rs

src/tools/miri/tests/fail/intrinsics/typed-swap-invalid-array.rs

bors · 2024-03-23T05:34:58Z

💔 Test failed - checks-actions

scottmcm · 2024-03-23T07:23:48Z

@bors r=oli-obk

bors · 2024-03-23T07:23:50Z

📌 Commit 75d2e5b has been approved by oli-obk

It is now in the queue for this repository.

bors · 2024-03-23T09:15:13Z

⌛ Testing commit 75d2e5b with merge ea4d0e9...

Let codegen decide when to `mem::swap` with immediates Making `libcore` decide this is silly; the backend has so much better information about when it's a good idea. Thus this PR introduces a new `typed_swap` intrinsic with a fallback body, and replaces that fallback implementation when swapping immediates or scalar pairs. r? oli-obk Replaces rust-lang#111744, and means we'll never need more libs PRs like rust-lang#111803 or rust-lang#107140

bors · 2024-03-23T13:30:13Z

💥 Test timed out

rust-log-analyzer · 2024-03-23T13:34:40Z

A job failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)

RalfJung · 2024-03-23T13:42:46Z

@bors retry

bors · 2024-03-23T13:57:58Z

⌛ Testing commit 75d2e5b with merge d6eb0f5...

bors · 2024-03-23T15:57:11Z

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

rust-timer · 2024-03-23T17:14:38Z

Finished benchmarking commit (d6eb0f5): 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.4%	[-0.4%, -0.4%]	1
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-0.4%	[-0.4%, -0.4%]	1

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)	4.3%	[0.1%, 6.8%]	3
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-5.3%	[-5.3%, -5.3%]	1
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	1.9%	[-5.3%, 6.8%]	4

Cycles

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)	2.5%	[2.5%, 2.5%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-	-	0

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.1%	[0.0%, 0.4%]	27
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-0.2%	[-1.0%, -0.0%]	10
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	0.0%	[-1.0%, 0.4%]	37

Bootstrap: 669.588s -> 671.198s (0.24%)
Artifact size: 315.05 MiB -> 314.95 MiB (-0.03%)

Let codegen decide when to `mem::swap` with immediates Making `libcore` decide this is silly; the backend has so much better information about when it's a good idea. Thus this PR introduces a new `typed_swap` intrinsic with a fallback body, and replaces that fallback implementation when swapping immediates or scalar pairs. r? oli-obk Replaces rust-lang#111744, and means we'll never need more libs PRs like rust-lang#111803 or rust-lang#107140

rustbot assigned oli-obk Mar 16, 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 Mar 16, 2024

This comment has been minimized.

Sign in to view

scottmcm marked this pull request as draft March 16, 2024 05:45

scottmcm 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-review Status: Awaiting review from the assignee but also interested parties. labels Mar 16, 2024

scottmcm force-pushed the swap-intrinsic-v2 branch from 725417f to 9f992da Compare March 16, 2024 07:53

scottmcm commented Mar 16, 2024

View reviewed changes

src/tools/miri/tests/fail/intrinsics/typed-swap-invalid-array.rs Outdated Show resolved Hide resolved

scottmcm marked this pull request as ready for review March 16, 2024 09:23

scottmcm added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Mar 16, 2024

the8472 reviewed Mar 16, 2024

View reviewed changes

library/core/src/mem/mod.rs Outdated Show resolved Hide resolved

This comment has been minimized.

Sign in to view

rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Mar 16, 2024

This comment has been minimized.

Sign in to view

This comment was marked as outdated.

Sign in to view

rustbot added perf-regression Performance regression. and removed S-waiting-on-perf Status: Waiting on a perf run to be completed. labels Mar 16, 2024

RalfJung reviewed Mar 17, 2024

View reviewed changes

library/core/src/ptr/mod.rs Outdated Show resolved Hide resolved

RalfJung reviewed Mar 17, 2024

View reviewed changes

library/core/src/mem/mod.rs Outdated Show resolved Hide resolved

RalfJung reviewed Mar 17, 2024

View reviewed changes

compiler/rustc_const_eval/src/interpret/intrinsics.rs Outdated Show resolved Hide resolved

RalfJung reviewed Mar 17, 2024

View reviewed changes

src/tools/miri/tests/fail/intrinsics/typed-swap-invalid-array.rs Outdated Show resolved Hide resolved

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

Avoid non-windows non-linux in assembly x64 test

75d2e5b

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 23, 2024

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

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 23, 2024

bors added the merged-by-bors This PR was explicitly merged by bors. label Mar 23, 2024

bors merged commit d6eb0f5 into rust-lang:master Mar 23, 2024

rustbot added this to the 1.79.0 milestone Mar 23, 2024

This was referenced Mar 23, 2024

refactor check_{lang,library}_ub: use a single intrinsic #122629

Merged

CFI: Strip auto traits off Virtual calls #122879

Merged

scottmcm deleted the swap-intrinsic-v2 branch March 23, 2024 16:47

bors mentioned this pull request Mar 23, 2024

Rollup of 10 pull requests #122929

Closed

clubby789 mentioned this pull request Mar 24, 2024

Don't emit load metadata in debug mode #122849

Merged

scottmcm mentioned this pull request Dec 30, 2024

stabilize const_swap #134757

Merged

Uh oh!

Conversation

scottmcm commented Mar 16, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This comment has been minimized.

Uh oh!

rustbot commented Mar 16, 2024

Uh oh!

Uh oh!

scottmcm commented Mar 16, 2024

Uh oh!

This comment has been minimized.

bors commented Mar 16, 2024

Uh oh!

bors commented Mar 16, 2024

Uh oh!

This comment has been minimized.

This comment was marked as outdated.

Uh oh!

RalfJung Mar 17, 2024

Choose a reason for hiding this comment

Uh oh!

scottmcm Mar 17, 2024

Choose a reason for hiding this comment

Uh oh!

RalfJung Mar 17, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

bors commented Mar 23, 2024

Uh oh!

scottmcm commented Mar 23, 2024

Uh oh!

bors commented Mar 23, 2024

Uh oh!

bors commented Mar 23, 2024

Uh oh!

bors commented Mar 23, 2024

Uh oh!

rust-log-analyzer commented Mar 23, 2024

Uh oh!

RalfJung commented Mar 23, 2024

Uh oh!

bors commented Mar 23, 2024

Uh oh!

bors commented Mar 23, 2024

Uh oh!

rust-timer commented Mar 23, 2024

Overall result: ✅ improvements - no action needed

Instruction count

Max RSS (memory usage)

Cycles

Binary size

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

scottmcm commented Mar 16, 2024 •

edited

Loading