Micro-optimize `InstSimplify`'s `simplify_primitive_clone` #139644

yotamofek · 2025-04-10T18:10:07Z

r? @compiler-errors , since you already did #139411 and got randomly selected for #139638 (feel free to reassign!)

Another one similar in spirit to #139411, but this time for simplify_primitive_clone, which is doing a bit of redundant work. Might not show up in benches, but probably worth micro-optimizing since the transformation is run even for debug builds.

See inline comments for my reasoning for making these changes.

rustbot · 2025-04-10T18:10:13Z

Some changes occurred to MIR optimizations

cc @rust-lang/wg-mir-opt

yotamofek · 2025-04-10T18:10:42Z

compiler/rustc_mir_transform/src/instsimplify.rs

-        let Some((fn_def_id, fn_args)) = func.const_fn_def() else { return };
-
-        // Clone needs one arg, so we can cheaply rule out other stuff
-        if fn_args.len() != 1 {


No need for second check that the fn being called has only 1 arg

I can maybe leave a debug_assert here as a sanity check?

It's fine, we'll ICE otherwise or at least fail MIR validation.

compiler/rustc_mir_transform/src/instsimplify.rs

yotamofek · 2025-04-10T18:13:27Z

compiler/rustc_mir_transform/src/instsimplify.rs

            return;
        }

-        let Ok([arg]) = take_array(args) else { return };


No need to call take_array(args), since we're overriding terminator.kind later on.
Actually, I think it might also leave args in an invalid state if we early-return on the next line, but haven't been able to exercise that code path.
Allows taking an immutable ref to &terminator.kind at the beginning of the function.

Also, this can never return Err, we already checked that args's len is 1

compiler-errors · 2025-04-10T19:18:14Z

@bors try @rust-timer queue

…ify/simplify_primitive_clone, r=<try> Micro-optimize `InstSimplify`'s `simplify_primitive_clone` r? `@compiler-errors` , since you already did rust-lang#139411 and got randomly selected for rust-lang#139638 (feel free to reassign!) Another one similar in spirit to rust-lang#139411, but this time for `simplify_primitive_clone`, which is doing a bit of redundant work. Might not show up in benches, but probably worth micro-optimizing since the transformation is run even for debug builds. See inline comments for my reasoning for making these changes.

bors · 2025-04-10T19:19:25Z

⌛ Trying commit 0069cad with merge 7ddd244...

bors · 2025-04-10T21:17:28Z

☀️ Try build successful - checks-actions
Build commit: 7ddd244 (7ddd244265505d97dc8a6039f123c88b03fe31a2)

rust-timer · 2025-04-11T05:15:23Z

Finished benchmarking commit (7ddd244): comparison URL.

Overall result: ❌ regressions - no action needed

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-perf -perf-regression

Instruction count

This is the most reliable metric that we have; it was used to determine the overall result at the top of this comment. However, even this metric can sometimes exhibit noise.

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

Max RSS (memory usage)

Results (primary -2.6%)

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

Cycles

Results (secondary 4.4%)

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)	4.4%	[3.7%, 5.0%]	2
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-	-	0

Binary size

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

Bootstrap: 782.988s -> 783.278s (0.04%)
Artifact size: 366.24 MiB -> 366.30 MiB (0.02%)

compiler-errors · 2025-04-11T05:16:33Z

Noise probably

compiler-errors · 2025-04-13T16:28:12Z

@bors r+ rollup

bors · 2025-04-13T16:28:14Z

📌 Commit 0069cad has been approved by compiler-errors

It is now in the queue for this repository.

…plify/simplify_primitive_clone, r=compiler-errors Micro-optimize `InstSimplify`'s `simplify_primitive_clone` r? `@compiler-errors` , since you already did rust-lang#139411 and got randomly selected for rust-lang#139638 (feel free to reassign!) Another one similar in spirit to rust-lang#139411, but this time for `simplify_primitive_clone`, which is doing a bit of redundant work. Might not show up in benches, but probably worth micro-optimizing since the transformation is run even for debug builds. See inline comments for my reasoning for making these changes.

…plify/simplify_primitive_clone, r=compiler-errors Micro-optimize `InstSimplify`'s `simplify_primitive_clone` r? ``@compiler-errors`` , since you already did rust-lang#139411 and got randomly selected for rust-lang#139638 (feel free to reassign!) Another one similar in spirit to rust-lang#139411, but this time for `simplify_primitive_clone`, which is doing a bit of redundant work. Might not show up in benches, but probably worth micro-optimizing since the transformation is run even for debug builds. See inline comments for my reasoning for making these changes.

…enton Rollup of 12 pull requests Successful merges: - rust-lang#138744 (Add methods to TCP and UDP sockets to modify hop limit (refresh of rust-lang#94678)) - rust-lang#138962 (Expect an array when expected and acutal types are both arrays during cast) - rust-lang#139001 (add `naked_functions_rustic_abi` feature gate) - rust-lang#139379 (Use delayed bug for normalization errors in drop elaboration) - rust-lang#139582 (Various coercion cleanups) - rust-lang#139628 (Suggest remove redundant `$()?` around `vis`) - rust-lang#139644 (Micro-optimize `InstSimplify`'s `simplify_primitive_clone`) - rust-lang#139666 (cleanup `mir_borrowck`) - rust-lang#139674 (In `rustc_mir_transform`, iterate over index newtypes instead of ints) - rust-lang#139695 (compiletest: consistently use `camino::{Utf8Path,Utf8PathBuf}` throughout) - rust-lang#139722 (Move some things to rustc_type_ir) - rust-lang#139740 (Convert `tests/ui/lint/dead-code/self-assign.rs` to a known-bug test) r? `@ghost` `@rustbot` modify labels: rollup

…plify/simplify_primitive_clone, r=compiler-errors Micro-optimize `InstSimplify`'s `simplify_primitive_clone` r? ```@compiler-errors``` , since you already did rust-lang#139411 and got randomly selected for rust-lang#139638 (feel free to reassign!) Another one similar in spirit to rust-lang#139411, but this time for `simplify_primitive_clone`, which is doing a bit of redundant work. Might not show up in benches, but probably worth micro-optimizing since the transformation is run even for debug builds. See inline comments for my reasoning for making these changes.

Rollup of 11 pull requests Successful merges: - rust-lang#137043 (Initial `UnsafePinned` implementation [Part 1: Libs]) - rust-lang#138962 (Expect an array when expected and acutal types are both arrays during cast) - rust-lang#139001 (add `naked_functions_rustic_abi` feature gate) - rust-lang#139379 (Use delayed bug for normalization errors in drop elaboration) - rust-lang#139582 (Various coercion cleanups) - rust-lang#139628 (Suggest remove redundant `$()?` around `vis`) - rust-lang#139644 (Micro-optimize `InstSimplify`'s `simplify_primitive_clone`) - rust-lang#139671 (Proc macro span API redesign: Replace proc_macro::SourceFile by Span::{file, local_file}) - rust-lang#139674 (In `rustc_mir_transform`, iterate over index newtypes instead of ints) - rust-lang#139740 (Convert `tests/ui/lint/dead-code/self-assign.rs` to a known-bug test) - rust-lang#139741 (fix smir's run! doc and import) r? `@ghost` `@rustbot` modify labels: rollup

Rollup of 10 pull requests Successful merges: - rust-lang#137043 (Initial `UnsafePinned` implementation [Part 1: Libs]) - rust-lang#138962 (Expect an array when expected and acutal types are both arrays during cast) - rust-lang#139001 (add `naked_functions_rustic_abi` feature gate) - rust-lang#139379 (Use delayed bug for normalization errors in drop elaboration) - rust-lang#139582 (Various coercion cleanups) - rust-lang#139628 (Suggest remove redundant `$()?` around `vis`) - rust-lang#139644 (Micro-optimize `InstSimplify`'s `simplify_primitive_clone`) - rust-lang#139674 (In `rustc_mir_transform`, iterate over index newtypes instead of ints) - rust-lang#139740 (Convert `tests/ui/lint/dead-code/self-assign.rs` to a known-bug test) - rust-lang#139741 (fix smir's run! doc and import) r? `@ghost` `@rustbot` modify labels: rollup

Rollup merge of rust-lang#139644 - yotamofek:pr/mir_transform/instsimplify/simplify_primitive_clone, r=compiler-errors Micro-optimize `InstSimplify`'s `simplify_primitive_clone` r? ````@compiler-errors```` , since you already did rust-lang#139411 and got randomly selected for rust-lang#139638 (feel free to reassign!) Another one similar in spirit to rust-lang#139411, but this time for `simplify_primitive_clone`, which is doing a bit of redundant work. Might not show up in benches, but probably worth micro-optimizing since the transformation is run even for debug builds. See inline comments for my reasoning for making these changes.

Micro-optimize InstSimplify's simplify_primitive_clone

0069cad

rustbot assigned compiler-errors Apr 10, 2025

rustbot added S-waiting-on-review T-compiler labels Apr 10, 2025

yotamofek commented Apr 10, 2025

View reviewed changes

compiler/rustc_mir_transform/src/instsimplify.rs Show resolved Hide resolved

yotamofek commented Apr 10, 2025

View reviewed changes

This comment has been minimized.

Sign in to view

rustbot added the S-waiting-on-perf label Apr 10, 2025

This comment has been minimized.

Sign in to view

rustbot removed the S-waiting-on-perf label Apr 11, 2025

compiler-errors approved these changes Apr 13, 2025

View reviewed changes

bors added S-waiting-on-bors and removed S-waiting-on-review labels Apr 13, 2025

ChrisDenton mentioned this pull request Apr 13, 2025

Rollup of 13 pull requests #139754

Closed

ChrisDenton mentioned this pull request Apr 13, 2025

Rollup of 12 pull requests #139755

Closed

jhpratt mentioned this pull request Apr 13, 2025

Rollup of 11 pull requests #139761

Closed

jhpratt mentioned this pull request Apr 13, 2025

Rollup of 10 pull requests #139766

Merged

bors merged commit 388d612 into rust-lang:master Apr 14, 2025
7 checks passed

rustbot added this to the 1.88.0 milestone Apr 14, 2025

yotamofek deleted the pr/mir_transform/instsimplify/simplify_primitive_clone branch April 14, 2025 16:21

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Micro-optimize `InstSimplify`'s `simplify_primitive_clone` #139644

Micro-optimize `InstSimplify`'s `simplify_primitive_clone` #139644

yotamofek commented Apr 10, 2025 •

edited

Loading

rustbot commented Apr 10, 2025

yotamofek Apr 10, 2025

yotamofek Apr 10, 2025

compiler-errors Apr 13, 2025

yotamofek Apr 10, 2025

yotamofek Apr 10, 2025 •

edited

Loading

compiler-errors commented Apr 10, 2025

This comment has been minimized.

bors commented Apr 10, 2025

bors commented Apr 10, 2025

This comment has been minimized.

rust-timer commented Apr 11, 2025

compiler-errors commented Apr 11, 2025

compiler-errors commented Apr 13, 2025

bors commented Apr 13, 2025

Micro-optimize InstSimplify's simplify_primitive_clone #139644

Micro-optimize InstSimplify's simplify_primitive_clone #139644

Conversation

yotamofek commented Apr 10, 2025 • edited Loading

rustbot commented Apr 10, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yotamofek Apr 10, 2025 • edited Loading

Choose a reason for hiding this comment

compiler-errors commented Apr 10, 2025

This comment has been minimized.

bors commented Apr 10, 2025

bors commented Apr 10, 2025

This comment has been minimized.

rust-timer commented Apr 11, 2025

Overall result: ❌ regressions - no action needed

compiler-errors commented Apr 11, 2025

compiler-errors commented Apr 13, 2025

bors commented Apr 13, 2025

Micro-optimize `InstSimplify`'s `simplify_primitive_clone` #139644

Micro-optimize `InstSimplify`'s `simplify_primitive_clone` #139644

yotamofek commented Apr 10, 2025 •

edited

Loading

yotamofek Apr 10, 2025 •

edited

Loading