Avoid cloning jump threading state when possible #127024

cjgillot · 2024-06-27T08:25:31Z

The current implementation of jump threading passes most of its time cloning its state. This PR attempts to avoid such clones by special-casing the last predecessor when recursing through a terminator.

This is not optimal, but a first step while I refactor the state data structure to be sparse.

The two other commits are drive-by.
Fixes #116721

r? @oli-obk

rustbot · 2024-06-27T08:25:39Z

Some changes occurred to MIR optimizations

cc @rust-lang/wg-mir-opt

cjgillot · 2024-06-27T08:36:41Z

@bors try @rust-timer queue

Avoid cloning jump threading state when possible The current implementation of jump threading passes most of its time cloning its state. This PR attempts to avoid such clones by special-casing the last predecessor when recursing through a terminator. This is not optimal, but a first step while I refactor the state data structure to be sparse. The two other commits are drive-by. Fixes rust-lang#116721 r? `@oli-obk`

bors · 2024-06-27T08:37:52Z

⌛ Trying commit 271b9c2 with merge e8f9a6f...

oli-obk

r=me with ICE test removed (or moved to the UI test suite if you find it sufficiently useful)

lqd · 2024-06-27T09:52:44Z

The compile flags in the crash test are kinda sus, so here's another MCVE where only rustc -O is enough to ICE.

Minimized from the stable regression on lina in the original issue (playground):

pub struct NPlusOneArray<T>(T);

impl<T> AsRef<[T]> for NPlusOneArray<T>
where
    [T]: Copy,
{
    fn as_ref(&self) -> &[T] {
        todo!()
    }
}

bors · 2024-06-27T10:19:53Z

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

lqd · 2024-06-27T10:24:11Z

compiler/rustc_mir_dataflow/src/value_analysis.rs

@@ -846,9 +846,10 @@ impl Map {

        if let ty::Ref(_, ref_ty, _) | ty::RawPtr(ref_ty, _) = ty.kind()
            && let ty::Slice(..) = ref_ty.kind()
+            // The user may have written a predicate like `[T]: Sized` in their where claures,


Suggested change

// The user may have written a predicate like `[T]: Sized` in their where claures,

// The user may have written a predicate like `[T]: Sized` in their where clauses,

lqd · 2024-06-27T10:26:09Z

compiler/rustc_mir_transform/src/jump_threading.rs

+        self.find_opportunity(bb, state, cost, 0);
+        None
+    }
+
    /// Recursion entry point to find threading opportunities.


find_opportunity is not the entry-point anymore, right?

Suggested change

/// Recursion entry point to find threading opportunities.

Make jump threading state sparse Continuation of rust-lang#127024 Both dataflow const-prop and jump threading involve cloning the state vector a lot. This PR replaces the data structure by a sparse vector, considering: - that jump threading state is typically very sparse (at most 1 or 2 set entries); - that dataflow const-prop is disabled by default; - that place/value map is very eager, and prone to creating an overly large state. The first commit is shared with the previous PR to avoid needless conflicts. r? `@oli-obk`

rust-timer · 2024-06-27T12:50:59Z

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

Max RSS (memory usage)

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

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: 694.809s -> 696.87s (0.30%)
Artifact size: 326.69 MiB -> 326.77 MiB (0.03%)

cjgillot · 2024-06-29T10:44:27Z

The compile flags in the crash test are kinda sus, so here's another MCVE where only rustc -O is enough to ICE.

That's not very important, as this PR removed the assertion that used to trigger the ICE. For the future, what matters is remembering that [T]: Sized clauses can exist.

cjgillot · 2024-06-29T10:44:41Z

@bors r=oli-obk

Avoid cloning jump threading state when possible The current implementation of jump threading passes most of its time cloning its state. This PR attempts to avoid such clones by special-casing the last predecessor when recursing through a terminator. This is not optimal, but a first step while I refactor the state data structure to be sparse. The two other commits are drive-by. Fixes rust-lang#116721 r? `@oli-obk`

bors · 2024-06-30T05:53:01Z

⌛ Testing commit c81481f with merge 29eb5a0...

rust-log-analyzer · 2024-06-30T07:18:57Z

The job x86_64-msvc-ext failed! Check out the build log: (web) (plain)

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

[RUSTC-TIMING] miri test:false 63.376
[RUSTC-TIMING] miri test:false 3.047
    Finished `release` profile [optimized] target(s) in 0.39s
thread 'main' panicked at src/lib.rs:1697:17:
failed to copy `C:\a\rust\rust\build\x86_64-pc-windows-msvc\stage1-tools\x86_64-pc-windows-msvc\release\miri.exe` to `C:\a\rust\rust\build\x86_64-pc-windows-msvc\stage1-tools-bin\miri.exe`: The process cannot access the file because it is being used by another process. (os error 32)
##[endgroup]
Build completed unsuccessfully in 0:00:11
  local time: Sun, Jun 30, 2024  7:18:43 AM
  network time: Sun, 30 Jun 2024 07:18:43 GMT

bors · 2024-06-30T07:19:32Z

💔 Test failed - checks-actions

Kobzol · 2024-06-30T07:30:59Z

Spurious error.

@bors retry

bors · 2024-06-30T08:15:26Z

⌛ Testing commit c81481f with merge 8e372cc...

Avoid cloning jump threading state when possible The current implementation of jump threading passes most of its time cloning its state. This PR attempts to avoid such clones by special-casing the last predecessor when recursing through a terminator. This is not optimal, but a first step while I refactor the state data structure to be sparse. The two other commits are drive-by. Fixes rust-lang#116721 r? `@oli-obk`

rust-log-analyzer · 2024-06-30T09:41:02Z

The job x86_64-msvc-ext failed! Check out the build log: (web) (plain)

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

[RUSTC-TIMING] miri test:false 62.837
[RUSTC-TIMING] miri test:false 2.945
    Finished `release` profile [optimized] target(s) in 0.39s
thread 'main' panicked at src/lib.rs:1697:17:
failed to copy `C:\a\rust\rust\build\x86_64-pc-windows-msvc\stage1-tools\x86_64-pc-windows-msvc\release\miri.exe` to `C:\a\rust\rust\build\x86_64-pc-windows-msvc\stage1-tools-bin\miri.exe`: The process cannot access the file because it is being used by another process. (os error 32)
##[endgroup]
Build completed unsuccessfully in 0:00:08
  local time: Sun, Jun 30, 2024  9:40:47 AM
  network time: Sun, 30 Jun 2024 09:40:47 GMT

bors · 2024-06-30T09:41:35Z

💔 Test failed - checks-actions

cjgillot · 2024-06-30T09:42:31Z

@bors retry

bors · 2024-06-30T11:09:56Z

⌛ Testing commit c81481f with merge 2975a21...

bors · 2024-06-30T13:21:36Z

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

rust-timer · 2024-06-30T15:01:10Z

Finished benchmarking commit (2975a21): comparison URL.

Overall result: ❌ regressions - 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)	1.6%	[1.6%, 1.6%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-	-	0

Max RSS (memory usage)

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

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: 695.533s -> 695.757s (0.03%)
Artifact size: 324.63 MiB -> 324.71 MiB (0.03%)

Make jump threading state sparse Continuation of rust-lang#127024 Both dataflow const-prop and jump threading involve cloning the state vector a lot. This PR replaces the data structure by a sparse vector, considering: - that jump threading state is typically very sparse (at most 1 or 2 set entries); - that dataflow const-prop is disabled by default; - that place/value map is very eager, and prone to creating an overly large state. The first commit is shared with the previous PR to avoid needless conflicts. r? `@oli-obk`

Make jump threading state sparse Continuation of rust-lang/rust#127024 Both dataflow const-prop and jump threading involve cloning the state vector a lot. This PR replaces the data structure by a sparse vector, considering: - that jump threading state is typically very sparse (at most 1 or 2 set entries); - that dataflow const-prop is disabled by default; - that place/value map is very eager, and prone to creating an overly large state. The first commit is shared with the previous PR to avoid needless conflicts. r? `@oli-obk`

rustbot assigned oli-obk Jun 27, 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. labels Jun 27, 2024

This comment has been minimized.

Sign in to view

cjgillot force-pushed the jump-prof branch from b959a23 to 271b9c2 Compare June 27, 2024 08:36

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 Jun 27, 2024

This comment has been minimized.

Sign in to view

oli-obk approved these changes Jun 27, 2024

View reviewed changes

This comment has been minimized.

Sign in to view

lqd reviewed Jun 27, 2024

View reviewed changes

cjgillot mentioned this pull request Jun 27, 2024

Make jump threading state sparse #127036

Merged

rustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Jun 27, 2024

cjgillot added the A-mir-opt Area: MIR optimizations label Jun 28, 2024

cjgillot added 4 commits June 29, 2024 10:42

Stop ICEing on impossible predicates.

61ede07

Avoid cloning state when possible.

a175817

Move entry point to a method.

dec4e98

Move crash test.

c81481f

cjgillot force-pushed the jump-prof branch from 8e4ba83 to c81481f Compare June 29, 2024 10:42

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 Jun 30, 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 Jun 30, 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 Jun 30, 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 Jun 30, 2024

bors added the merged-by-bors This PR was explicitly merged by bors. label Jun 30, 2024

bors merged commit 2975a21 into rust-lang:master Jun 30, 2024
7 checks passed

rustbot added this to the 1.81.0 milestone Jun 30, 2024

cjgillot deleted the jump-prof branch June 30, 2024 13:26

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Avoid cloning jump threading state when possible #127024

Avoid cloning jump threading state when possible #127024

cjgillot commented Jun 27, 2024

rustbot commented Jun 27, 2024

This comment has been minimized.

cjgillot commented Jun 27, 2024

This comment has been minimized.

bors commented Jun 27, 2024

This comment has been minimized.

oli-obk left a comment

lqd commented Jun 27, 2024

bors commented Jun 27, 2024

This comment has been minimized.

lqd Jun 27, 2024

lqd Jun 27, 2024

rust-timer commented Jun 27, 2024

cjgillot commented Jun 29, 2024

cjgillot commented Jun 29, 2024

bors commented Jun 30, 2024

rust-log-analyzer commented Jun 30, 2024

bors commented Jun 30, 2024

Kobzol commented Jun 30, 2024

bors commented Jun 30, 2024

rust-log-analyzer commented Jun 30, 2024

bors commented Jun 30, 2024

cjgillot commented Jun 30, 2024

bors commented Jun 30, 2024

bors commented Jun 30, 2024

rust-timer commented Jun 30, 2024

	// The user may have written a predicate like `[T]: Sized` in their where claures,
	// The user may have written a predicate like `[T]: Sized` in their where clauses,

Avoid cloning jump threading state when possible #127024

Avoid cloning jump threading state when possible #127024

Conversation

cjgillot commented Jun 27, 2024

rustbot commented Jun 27, 2024

This comment has been minimized.

cjgillot commented Jun 27, 2024

This comment has been minimized.

bors commented Jun 27, 2024

This comment has been minimized.

oli-obk left a comment

Choose a reason for hiding this comment

lqd commented Jun 27, 2024

bors commented Jun 27, 2024

This comment has been minimized.

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rust-timer commented Jun 27, 2024

Overall result: ❌ regressions - no action needed

cjgillot commented Jun 29, 2024

cjgillot commented Jun 29, 2024

bors commented Jun 30, 2024

rust-log-analyzer commented Jun 30, 2024

bors commented Jun 30, 2024

Kobzol commented Jun 30, 2024

bors commented Jun 30, 2024

rust-log-analyzer commented Jun 30, 2024

bors commented Jun 30, 2024

cjgillot commented Jun 30, 2024

bors commented Jun 30, 2024

bors commented Jun 30, 2024

rust-timer commented Jun 30, 2024

Overall result: ❌ regressions - no action needed