Prefer -1 for None

[scottmcm]

View all comments

Currently we pick "weird" numbers like 1114112 for None::<char>. While that's not wrong, it's kinda unnatural -- a human wouldn't make that choice.

This PR instead picks -1 for thinge like None::<char> -- like clang's WEOF -- and None::<bool> and such.

Any enums with more than one niched value (so not Result nor Option) remain as they were before. Also we continue to use 0 when that's possible -- -1 is only preferred when zero isn't possible.

---

Inspired when someone in discord posted an example like this https://rust.godbolt.org/z/W94s9qdYW and I thought it was odd that we're currently picking -9223372036854775808 to be the value to store to mark an Option<Vec<_>> as None. (Especially since that needs an 8-byte immediate on x64, and writing -1 is only a 4-byte immediate.)

[scottmcm]

Note that this arm just kinda gave up before; it never even considered whether putting it next to start might be better.

View changes since the review

[scottmcm]

@bors try @rust-timer queue

[scottmcm]

It helps to remember to git add all the changes 🤦

[scottmcm]

@bors try @rust-timer queue

[rust-bors]

☀️ Try build successful (CI)
Build commit: ed76e05 (ed76e056cbb36453c64da4dd8ffccac4e093f819, parent: 2f201bccb3a7fb5a85b0fcfcc0a020a946d6d58a)

[rust-timer]

Finished benchmarking commit (ed76e05): comparison URL.

Overall result: ❌✅ regressions and improvements - please read:

Benchmarking means the PR may be perf-sensitive. It's automatically marked not fit for rolling up. Overriding is possible but disadvised: it risks changing compiler perf.

Next, please: If you can, justify the regressions found in this try perf run in writing along with @rustbot label: +perf-regression-triaged. If not, fix the regressions and do another perf run. Neutral or positive results will clear the label automatically.

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

Instruction count

Our most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.

	mean	range	count
Regressions ❌ (primary)	0.3%	[0.3%, 0.3%]	1
Regressions ❌ (secondary)	0.5%	[0.0%, 1.5%]	9
Improvements ✅ (primary)	-0.3%	[-2.9%, -0.2%]	86
Improvements ✅ (secondary)	-0.3%	[-1.8%, -0.0%]	62
All ❌✅ (primary)	-0.3%	[-2.9%, 0.3%]	87

Max RSS (memory usage)

Results (secondary 1.1%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

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

Cycles

Results (primary -2.9%, secondary -4.2%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	1.8%	[1.8%, 1.8%]	1
Improvements ✅ (primary)	-2.9%	[-3.0%, -2.9%]	2
Improvements ✅ (secondary)	-6.2%	[-7.6%, -3.4%]	3
All ❌✅ (primary)	-2.9%	[-3.0%, -2.9%]	2

Binary size

Results (primary -0.2%, secondary -0.4%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	0.3%	[0.0%, 0.6%]	2
Improvements ✅ (primary)	-0.2%	[-0.6%, -0.1%]	5
Improvements ✅ (secondary)	-0.9%	[-2.1%, -0.3%]	3
All ❌✅ (primary)	-0.2%	[-0.6%, -0.1%]	5

Bootstrap: 491.618s -> 493.312s (0.34%)
Artifact size: 394.25 MiB -> 396.31 MiB (0.52%)

[scottmcm]

Wow, those results are way better than I expected 😅

Seems pretty clearly a net win to me.
@rustbot label: +perf-regression-triaged

[rustbot]

r? @chenyukang

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

Why was this reviewer chosen?

The reviewer was selected based on:

• Owners of files modified in this PR: compiler
• compiler expanded to 72 candidates
• Random selection from 18 candidates

[scottmcm]

@bors try jobs=x86_64-gnu-llvm-22-1,x86_64-gnu-llvm-21-1

[scottmcm]

Oh no, the test passed in LLVM21

but fails in LLVM22

Also, it passes in stage1 but fails in stage2 😬

---

For extra hilarity, we have a test for Option<bool>::eq specifically

rust/tests/codegen-llvm/option-niche-eq.rs

Lines 77 to 84 in 5095b44

	// CHECK-LABEL: @bool_eq
	#[no_mangle]
	pub fn bool_eq(l: Option<bool>, r: Option<bool>) -> bool {
	// CHECK: start:
	// CHECK-NEXT: icmp eq i8
	// CHECK-NEXT: ret i1
	l == r
	}

and that passes on both stage1 and stage2

[rust-bors]

💔 Test for fdc4b82 failed: CI. Failed job:

• try - x86_64-gnu-llvm-22-1 (web logs, enhanced plaintext logs)

[rustbot]

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

[scottmcm]

@bors try jobs=x86_64-gnu-llvm-22-1,x86_64-gnu-llvm-21-1

[rust-bors]

☀️ Try build successful (CI)
Build commit: 8cc9601 (8cc96016b66fa2bc1fd69714bf56ab3cfd979d6c, parent: 9838411cb723b60dc62b1625751075c4d933b992)

[scottmcm]

@bors try @rust-timer queue

[rust-bors]

☀️ Try build successful (CI)
Build commit: a6faa48 (a6faa48f4ec2a7e8550bf217c63861e52365583d, parent: 9838411cb723b60dc62b1625751075c4d933b992)

[rust-timer]

Finished benchmarking commit (a6faa48): comparison URL.

Overall result: ✅ improvements - no action needed

Benchmarking means the PR may be perf-sensitive. It's automatically marked not fit for rolling up. Overriding is possible but disadvised: it risks changing compiler perf.

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

Instruction count

Our most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	0.8%	[0.4%, 1.9%]	4
Improvements ✅ (primary)	-0.3%	[-2.7%, -0.2%]	57
Improvements ✅ (secondary)	-0.4%	[-3.1%, -0.2%]	47
All ❌✅ (primary)	-0.3%	[-2.7%, -0.2%]	57

Max RSS (memory usage)

Results (secondary 3.9%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	7.0%	[5.5%, 8.5%]	2
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-2.1%	[-2.1%, -2.1%]	1
All ❌✅ (primary)	-	-	0

Cycles

Results (primary 1.1%, secondary 1.2%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	3.1%	[2.4%, 3.7%]	2
Regressions ❌ (secondary)	4.2%	[2.1%, 6.3%]	2
Improvements ✅ (primary)	-2.8%	[-2.8%, -2.8%]	1
Improvements ✅ (secondary)	-4.7%	[-4.7%, -4.7%]	1
All ❌✅ (primary)	1.1%	[-2.8%, 3.7%]	3

Binary size

Results (primary -0.2%, secondary -0.1%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	0.0%	[0.0%, 0.0%]	2
Improvements ✅ (primary)	-0.2%	[-0.6%, -0.0%]	23
Improvements ✅ (secondary)	-0.1%	[-0.3%, -0.0%]	6
All ❌✅ (primary)	-0.2%	[-0.6%, -0.0%]	23

Bootstrap: 486.61s -> 487.264s (0.13%)
Artifact size: 394.05 MiB -> 393.66 MiB (-0.10%)