Reoptimize layout array #99174

scottmcm · 2022-07-12T10:12:43Z

This way it's one check instead of two, so hopefully (cc #99117) it'll be simpler for rustc perf too 🤞

Quick demonstration:

pub fn demo(n: usize) -> Option<Layout> {
    Layout::array::<i32>(n).ok()
}

Nightly: https://play.rust-lang.org/?version=nightly&mode=release&edition=2021&gist=e97bf33508aa03f38968101cdeb5322d

	mov	rax, rdi
	mov	ecx, 4
	mul	rcx
	seto	cl
	movabs	rdx, 9223372036854775805
	xor	esi, esi
	cmp	rax, rdx
	setb	sil
	shl	rsi, 2
	xor	edx, edx
	test	cl, cl
	cmove	rdx, rsi
	ret

This PR (note no mul, in addition to being much shorter):

	xor	edx, edx
	lea	rax, [4*rcx]
	shr	rcx, 61
	sete	dl
	shl	rdx, 2
	ret

This is built atop @CAD97 's #99136; the new changes are cb8aba66ef6a0e17f08a0574e4820653e31b45a0.

I added a bunch more tests for Layout::from_size_align and Layout::array too.

rust-highfive · 2022-07-12T10:12:49Z

r? @kennytm

(rust-highfive has picked a reviewer for you, use r? to override)

scottmcm · 2022-07-12T10:26:00Z

@bors try @rust-timer queue

rust-timer · 2022-07-12T10:26:02Z

Awaiting bors try build completion.

@rustbot label: +S-waiting-on-perf

bors · 2022-07-12T10:26:08Z

⌛ Trying commit cb8aba66ef6a0e17f08a0574e4820653e31b45a0 with merge c664874490fa0b8cc67066d620a5b53d8ed52cc9...

bors · 2022-07-12T11:58:39Z

☀️ Try build successful - checks-actions
Build commit: c664874490fa0b8cc67066d620a5b53d8ed52cc9 (c664874490fa0b8cc67066d620a5b53d8ed52cc9)

rust-timer · 2022-07-12T11:58:41Z

Queued c664874490fa0b8cc67066d620a5b53d8ed52cc9 with parent b3f4c31, future comparison URL.

rust-timer · 2022-07-12T13:14:16Z

Finished benchmarking commit (c664874490fa0b8cc67066d620a5b53d8ed52cc9): comparison url.

Instruction count

Primary benchmarks: 🎉 relevant improvements found
Secondary benchmarks: 🎉 relevant improvements found

	mean¹	max	count²
Regressions 😿 (primary)	N/A	N/A	0
Regressions 😿 (secondary)	N/A	N/A	0
Improvements 🎉 (primary)	-0.5%	-1.4%	12
Improvements 🎉 (secondary)	-0.9%	-1.3%	7
All 😿🎉 (primary)	-0.5%	-1.4%	12

Max RSS (memory usage)

Results

Primary benchmarks: mixed results
Secondary benchmarks: 🎉 relevant improvements found

	mean¹	max	count²
Regressions 😿 (primary)	4.0%	4.0%	1
Regressions 😿 (secondary)	N/A	N/A	0
Improvements 🎉 (primary)	-7.3%	-9.0%	3
Improvements 🎉 (secondary)	-1.7%	-2.1%	2
All 😿🎉 (primary)	-4.5%	-9.0%	4

Cycles

Results

Primary benchmarks: mixed results
Secondary benchmarks: no relevant changes found

	mean¹	max	count²
Regressions 😿 (primary)	2.1%	2.1%	1
Regressions 😿 (secondary)	N/A	N/A	0
Improvements 🎉 (primary)	-1.7%	-1.7%	1
Improvements 🎉 (secondary)	N/A	N/A	0
All 😿🎉 (primary)	0.2%	2.1%	2

If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf.

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. While you can manually mark this PR as fit for rollup, we strongly recommend not doing so since this PR may lead to changes in compiler perf.

@bors rollup=never
@rustbot label: +S-waiting-on-review -S-waiting-on-perf -perf-regression

the arithmetic mean of the percent change ↩ ↩² ↩³
number of relevant changes ↩ ↩² ↩³

CAD97

No measured regressions and reverses a few of the primary regressions from #95295; lgtm

library/core/tests/alloc.rs

This way it's one check instead of two, so hopefully it'll be better Nightly: ``` layout_array_i32: movq %rdi, %rax movl $4, %ecx mulq %rcx jo .LBB1_2 movabsq $9223372036854775805, %rcx cmpq %rcx, %rax jae .LBB1_2 movl $4, %edx retq .LBB1_2: … ``` This PR: ``` movq %rcx, %rax shrq $61, %rax jne .LBB2_1 shlq $2, %rcx movl $4, %edx movq %rcx, %rax retq .LBB2_1: … ```

scottmcm · 2022-07-14T00:09:13Z

Rebased atop the predecessor; might as well re-check perf just to be sure.

@bors try @rust-timer queue

rust-timer · 2022-07-14T00:09:14Z

Awaiting bors try build completion.

@rustbot label: +S-waiting-on-perf

bors · 2022-07-14T00:09:21Z

⌛ Trying commit a32305a with merge a28451281f53cbdfbe9f61b22b15a00d24a71a9a...

bors · 2022-07-14T01:36:43Z

☀️ Try build successful - checks-actions
Build commit: a28451281f53cbdfbe9f61b22b15a00d24a71a9a (a28451281f53cbdfbe9f61b22b15a00d24a71a9a)

rust-timer · 2022-07-14T01:36:45Z

Queued a28451281f53cbdfbe9f61b22b15a00d24a71a9a with parent 87588a2, future comparison URL.

rust-timer · 2022-07-14T02:52:19Z

Finished benchmarking commit (a28451281f53cbdfbe9f61b22b15a00d24a71a9a): comparison url.

Instruction count

Primary benchmarks: 🎉 relevant improvements found
Secondary benchmarks: 🎉 relevant improvements found

	mean¹	max	count²
Regressions 😿 (primary)	N/A	N/A	0
Regressions 😿 (secondary)	N/A	N/A	0
Improvements 🎉 (primary)	-0.5%	-1.1%	10
Improvements 🎉 (secondary)	-0.6%	-1.5%	10
All 😿🎉 (primary)	-0.5%	-1.1%	10

Max RSS (memory usage)

Results

Primary benchmarks: mixed results
Secondary benchmarks: mixed results

	mean¹	max	count²
Regressions 😿 (primary)	4.0%	4.0%	1
Regressions 😿 (secondary)	3.0%	5.5%	3
Improvements 🎉 (primary)	-6.2%	-9.1%	3
Improvements 🎉 (secondary)	-2.5%	-4.0%	5
All 😿🎉 (primary)	-3.7%	-9.1%	4

Cycles

Results

Primary benchmarks: 🎉 relevant improvement found
Secondary benchmarks: 🎉 relevant improvement found

	mean¹	max	count²
Regressions 😿 (primary)	N/A	N/A	0
Regressions 😿 (secondary)	N/A	N/A	0
Improvements 🎉 (primary)	-2.0%	-2.0%	1
Improvements 🎉 (secondary)	-3.6%	-3.6%	1
All 😿🎉 (primary)	-2.0%	-2.0%	1

If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf.

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. While you can manually mark this PR as fit for rollup, we strongly recommend not doing so since this PR may lead to changes in compiler perf.

@bors rollup=never
@rustbot label: +S-waiting-on-review -S-waiting-on-perf -perf-regression

the arithmetic mean of the percent change ↩ ↩² ↩³
number of relevant changes ↩ ↩² ↩³

CAD97 · 2022-07-14T04:36:37Z

Delta delta is within noise. Same as last time: this looks to be a small but measurable win.

scottmcm · 2022-08-10T00:51:25Z

Re-rolling for a new reviewer since it's been 4 weeks:
r? libs

joshtriplett · 2022-08-10T06:50:01Z

LGTM.

@bors r+ rollup=never

bors · 2022-08-10T06:50:03Z

📌 Commit a32305a has been approved by joshtriplett

It is now in the queue for this repository.

bors · 2022-08-10T08:50:31Z

⌛ Testing commit a32305a with merge b5fa4672c865a8f7a69de98f7d4f744bd0b576c5...

bors · 2022-08-10T13:05:31Z

💥 Test timed out

Noratrieb · 2022-08-10T13:39:14Z

@rust-lang/infra in case you aren't aware already, bors is not feeling well

rust-log-analyzer · 2022-08-10T13:41:37Z

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

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

scottmcm · 2022-08-10T18:05:40Z

@bors retry (x86_64-msvc-1 builder timed out)

bors · 2022-08-10T23:50:21Z

⌛ Testing commit a32305a with merge 908fc5b...

bors · 2022-08-11T02:34:49Z

☀️ Test successful - checks-actions
Approved by: joshtriplett
Pushing 908fc5b to master...

rust-timer · 2022-08-11T03:52:49Z

Finished benchmarking commit (908fc5b): comparison url.

Instruction count

Primary benchmarks: ✅ relevant improvements found
Secondary benchmarks: ✅ relevant improvements found

	mean¹	max	count²
Regressions ❌ (primary)	N/A	N/A	0
Regressions ❌ (secondary)	N/A	N/A	0
Improvements ✅ (primary)	-0.4%	-0.8%	10
Improvements ✅ (secondary)	-0.8%	-1.2%	5
All ❌✅ (primary)	-0.4%	-0.8%	10

Max RSS (memory usage)

Results

Primary benchmarks: mixed results
Secondary benchmarks: ❌ relevant regressions found

	mean¹	max	count²
Regressions ❌ (primary)	2.9%	2.9%	1
Regressions ❌ (secondary)	2.6%	3.8%	4
Improvements ✅ (primary)	-4.6%	-6.5%	4
Improvements ✅ (secondary)	N/A	N/A	0
All ❌✅ (primary)	-3.1%	-6.5%	5

Cycles

Results

Primary benchmarks: ❌ relevant regression found
Secondary benchmarks: no relevant changes found

	mean¹	max	count²
Regressions ❌ (primary)	2.4%	2.4%	1
Regressions ❌ (secondary)	N/A	N/A	0
Improvements ✅ (primary)	N/A	N/A	0
Improvements ✅ (secondary)	N/A	N/A	0
All ❌✅ (primary)	2.4%	2.4%	1

If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf.

@rustbot label: -perf-regression

the arithmetic mean of the percent change ↩ ↩² ↩³
number of relevant changes ↩ ↩² ↩³

rust-highfive assigned kennytm Jul 12, 2022

rustbot added the T-libs Relevant to the library team, which will review and decide on the PR/issue. label Jul 12, 2022

This comment was marked as resolved.

Sign in to view

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 12, 2022

rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Jul 12, 2022

rustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Jul 12, 2022

CAD97 approved these changes Jul 12, 2022

View reviewed changes

library/core/tests/alloc.rs Outdated Show resolved Hide resolved

library/core/tests/alloc.rs Outdated Show resolved Hide resolved

scottmcm force-pushed the reoptimize-layout-array branch from cb8aba6 to 63d4da9 Compare July 12, 2022 17:05

scottmcm force-pushed the reoptimize-layout-array branch from 63d4da9 to a32305a Compare July 14, 2022 00:08

rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Jul 14, 2022

rustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Jul 14, 2022

scottmcm mentioned this pull request Jul 16, 2022

Move comparison before umul.with.overflow by a constant llvm/llvm-project#56562

Open

rust-highfive assigned joshtriplett and unassigned kennytm Aug 10, 2022

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 Aug 10, 2022

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 Aug 10, 2022

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 Aug 10, 2022

bors added the merged-by-bors This PR was explicitly merged by bors. label Aug 11, 2022

bors merged commit 908fc5b into rust-lang:master Aug 11, 2022

rustbot added this to the 1.65.0 milestone Aug 11, 2022

scottmcm deleted the reoptimize-layout-array branch August 11, 2022 06:55

scottmcm mentioned this pull request Aug 22, 2022

[EXPERIMENT] Use wider types in Layout multiplication #100866

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Reoptimize layout array #99174

Reoptimize layout array #99174

scottmcm commented Jul 12, 2022 •

edited

Loading

This comment was marked as resolved.

rust-highfive commented Jul 12, 2022

scottmcm commented Jul 12, 2022

rust-timer commented Jul 12, 2022

bors commented Jul 12, 2022

bors commented Jul 12, 2022

rust-timer commented Jul 12, 2022

rust-timer commented Jul 12, 2022

CAD97 left a comment

scottmcm commented Jul 14, 2022

rust-timer commented Jul 14, 2022

bors commented Jul 14, 2022

bors commented Jul 14, 2022

rust-timer commented Jul 14, 2022

rust-timer commented Jul 14, 2022

CAD97 commented Jul 14, 2022

scottmcm commented Aug 10, 2022

joshtriplett commented Aug 10, 2022

bors commented Aug 10, 2022

bors commented Aug 10, 2022

bors commented Aug 10, 2022

Noratrieb commented Aug 10, 2022

rust-log-analyzer commented Aug 10, 2022

scottmcm commented Aug 10, 2022

bors commented Aug 10, 2022

bors commented Aug 11, 2022

rust-timer commented Aug 11, 2022

Reoptimize layout array #99174

Reoptimize layout array #99174

Conversation

scottmcm commented Jul 12, 2022 • edited Loading

This comment was marked as resolved.

rust-highfive commented Jul 12, 2022

scottmcm commented Jul 12, 2022

rust-timer commented Jul 12, 2022

bors commented Jul 12, 2022

bors commented Jul 12, 2022

rust-timer commented Jul 12, 2022

rust-timer commented Jul 12, 2022

Footnotes

CAD97 left a comment

Choose a reason for hiding this comment

scottmcm commented Jul 14, 2022

rust-timer commented Jul 14, 2022

bors commented Jul 14, 2022

bors commented Jul 14, 2022

rust-timer commented Jul 14, 2022

rust-timer commented Jul 14, 2022

Footnotes

CAD97 commented Jul 14, 2022

scottmcm commented Aug 10, 2022

joshtriplett commented Aug 10, 2022

bors commented Aug 10, 2022

bors commented Aug 10, 2022

bors commented Aug 10, 2022

Noratrieb commented Aug 10, 2022

rust-log-analyzer commented Aug 10, 2022

scottmcm commented Aug 10, 2022

bors commented Aug 10, 2022

bors commented Aug 11, 2022

rust-timer commented Aug 11, 2022

Footnotes

scottmcm commented Jul 12, 2022 •

edited

Loading