Local copy propagation #87126

tmiasko · 2021-07-14T13:30:23Z

No description provided.

rust-highfive · 2021-07-14T13:30:26Z

r? @petrochenkov

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

tmiasko · 2021-07-14T13:30:46Z

@bors try @rust-timer queue

rust-timer · 2021-07-14T13:30:47Z

Awaiting bors try build completion.

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

bors · 2021-07-14T13:30:56Z

⌛ Trying commit 70fa3c560bb5bf31d1a0f82cbafbb60a799a9b86 with merge 59eaa76686d3d30df4d2fb84728dd625827d7c19...

bors · 2021-07-14T14:54:58Z

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

rust-timer · 2021-07-14T14:55:00Z

Queued 59eaa76686d3d30df4d2fb84728dd625827d7c19 with parent a08f25a, future comparison URL.

petrochenkov · 2021-07-14T15:26:08Z

r? @jonas-schievink (based on github suggestion)

jonas-schievink · 2021-07-14T15:33:38Z

I wrote #76723, which this seems to be based on or inspired by, so r? @oli-obk

tmiasko · 2021-07-14T16:21:18Z

Yeah, as I mention in the commit message, the main idea is based on #76723. The differences are:

The local invalidation is detected using generation numbers that are verified before the propagation.
This implementation makes no effort to support move operands in favour of simplicity.

rust-timer · 2021-07-14T16:37:21Z

Finished benchmarking try commit (59eaa76686d3d30df4d2fb84728dd625827d7c19): comparison url.

Summary: This change led to significant mixed results 🤷 in compiler performance.

Large improvement in instruction counts (up to -6.2% on incr-unchanged builds of unicode_normalization-opt)
Moderate regression in instruction counts (up to 2.8% on incr-patched: b9b3e592dd cherry picked builds of style-servo-debug)

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 led to changes in compiler perf.

Next Steps: If you can justify the regressions found in this perf run, please indicate this with @rustbot label: +perf-regression-triaged along with sufficient written justification. If you cannot justify the regressions please fix the regressions and do another perf run. If the next run shows neutral or positive results, the label will be automatically removed.

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

tmiasko · 2021-07-14T17:12:11Z

@bors try @rust-timer queue

rust-timer · 2021-07-14T17:12:12Z

Awaiting bors try build completion.

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

bors · 2021-07-14T17:12:20Z

⌛ Trying commit f23d460b103d3fee31644545bbef49dbb6eaa9b4 with merge 47f3f074c5979a08f8b69a79bc3c72ae572a19d0...

compiler/rustc_mir/src/transform/copy_propagation.rs

shamatar · 2021-07-14T17:34:16Z

src/test/mir-opt/simplify_try.try_identity.DestinationPropagation.diff

+-         StorageLive(_6);                 // scope 0 at $DIR/simplify_try.rs:22:13: 22:14
+-         _6 = ((_3 as Err).0: i32);       // scope 0 at $DIR/simplify_try.rs:22:13: 22:14
+-         StorageLive(_8);                 // scope 2 at $DIR/simplify_try.rs:22:37: 22:50
+-         StorageLive(_9);                 // scope 2 at $DIR/simplify_try.rs:22:48: 22:49
+-         _9 = _6;                         // scope 2 at $DIR/simplify_try.rs:22:48: 22:49
+-         _8 = _9;                         // scope 5 at $DIR/simplify_try.rs:22:37: 22:50
+-         StorageDead(_9);                 // scope 2 at $DIR/simplify_try.rs:22:49: 22:50
+-         ((_0 as Err).0: i32) = _8;       // scope 6 at $DIR/simplify_try.rs:22:26: 22:51
+         nop;                             // scope 0 at $DIR/simplify_try.rs:22:13: 22:14
+         nop;                             // scope 0 at $DIR/simplify_try.rs:22:13: 22:14
+         nop;                             // scope 2 at $DIR/simplify_try.rs:22:37: 22:50
+         nop;                             // scope 2 at $DIR/simplify_try.rs:22:48: 22:49
+         nop;                             // scope 2 at $DIR/simplify_try.rs:22:48: 22:49
+         nop;                             // scope 5 at $DIR/simplify_try.rs:22:37: 22:50
+         nop;                             // scope 2 at $DIR/simplify_try.rs:22:49: 22:50
+         nop;                             // scope 6 at $DIR/simplify_try.rs:22:26: 22:51
+          discriminant(_0) = 1;            // scope 6 at $DIR/simplify_try.rs:22:26: 22:51
+-         StorageDead(_8);                 // scope 2 at $DIR/simplify_try.rs:22:50: 22:51
+-         StorageDead(_6);                 // scope 0 at $DIR/simplify_try.rs:22:50: 22:51
+-         StorageDead(_3);                 // scope 0 at $DIR/simplify_try.rs:24:6: 24:7


What would be a reason of such a large cumulative effect? As far as I can see it should be something like ((_0 as Err).0: i32) = ((_3 as Err).0: i32); if I correctly understood that this pass only works along the/in the blocks and not over the full body

Content of the diff file is entirely due to the work of destination propagation.

On the other hand, the meta diff, i.e., the diff between the diffs, is negative interaction between copy propagation turning move into copy where simplify arm identity expects a move. As a result, what what previously optimized by simplify arm identity now reaches the destination propagation.

bjorn3 · 2021-07-14T17:41:35Z

compiler/rustc_mir/src/util/borrowed.rs

+use rustc_middle::mir::*;
+
+/// Walks MIR to find all locals that have their address taken anywhere.
+pub fn ever_borrowed_locals(body: &Body<'_>) -> BitSet<Local> {


Could this be used in the ssa analysis of cg_ssa?

bors · 2021-07-18T18:01:35Z

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

rust-timer · 2021-07-18T18:01:36Z

Queued f965efafe8e043e1b43f46c629cd1b01d6429627 with parent 331da58, future comparison URL.

rust-timer · 2021-07-18T22:25:21Z

Finished benchmarking try commit (f965efafe8e043e1b43f46c629cd1b01d6429627): comparison url.

Summary: This change led to significant mixed results 🤷 in compiler performance.

Large improvement in instruction counts (up to -6.7% on incr-unchanged builds of unicode_normalization-opt)
Moderate regression in instruction counts (up to 2.3% on full builds of externs-debug)

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 led to changes in compiler perf.

Next Steps: If you can justify the regressions found in this perf run, please indicate this with @rustbot label: +perf-regression-triaged along with sufficient written justification. If you cannot justify the regressions please fix the regressions and do another perf run. If the next run shows neutral or positive results, the label will be automatically removed.

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

bors · 2021-07-27T16:41:05Z

☔ The latest upstream changes (presumably #87509) made this pull request unmergeable. Please resolve the merge conflicts.

Based on ideas from 76723.

tmiasko · 2021-07-30T13:00:03Z

@bors try

bors · 2021-07-30T13:00:17Z

⌛ Trying commit ba76f5e8ce3c78bce3ef8e8b2b63565956fc4733 with merge 5239e46af8c42a7c122cff505a9f44a3a5fb44c5...

bors · 2021-07-30T13:48:01Z

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

tmiasko · 2021-07-30T13:51:56Z

@rust-timer build 5239e46af8c42a7c122cff505a9f44a3a5fb44c5
@bors try

rust-timer · 2021-07-30T13:51:58Z

Queued 5239e46af8c42a7c122cff505a9f44a3a5fb44c5 with parent 1195bea, future comparison URL.

bors · 2021-07-30T13:52:05Z

⌛ Trying commit b7fe965ea9868b2492cae521d49ebb43198b8849 with merge bd1ba56000b265b08adc0ce2d7a7e2f696a53214...

bors · 2021-07-30T14:38:59Z

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

rust-timer · 2021-07-30T15:35:47Z

Finished benchmarking try commit (5239e46af8c42a7c122cff505a9f44a3a5fb44c5): comparison url.

Summary: This change led to significant mixed results 🤷 in compiler performance.

Large regression in instruction counts (up to 5.8% on full builds of ripgrep-opt)
Moderate improvement in instruction counts (up to -4.4% on incr-unchanged builds of deeply-nested-async-check)

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 led to changes in compiler perf.

Next Steps: If you can justify the regressions found in this perf run, please indicate this with @rustbot label: +perf-regression-triaged along with sufficient written justification. If you cannot justify the regressions please fix the regressions and do another perf run. If the next run shows neutral or positive results, the label will be automatically removed.

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

tmiasko · 2021-07-30T15:36:51Z

@rust-timer build bd1ba56000b265b08adc0ce2d7a7e2f696a53214

rust-timer · 2021-07-30T15:36:52Z

Queued bd1ba56000b265b08adc0ce2d7a7e2f696a53214 with parent 1195bea, future comparison URL.

rust-timer · 2021-07-30T19:00:13Z

Finished benchmarking try commit (bd1ba56000b265b08adc0ce2d7a7e2f696a53214): comparison url.

Summary: This change led to significant mixed results 🤷 in compiler performance.

Large improvement in instruction counts (up to -6.3% on incr-unchanged builds of unicode_normalization-opt)
Large regression in instruction counts (up to 5.7% on full builds of ripgrep-opt)

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 led to changes in compiler perf.

Next Steps: If you can justify the regressions found in this perf run, please indicate this with @rustbot label: +perf-regression-triaged along with sufficient written justification. If you cannot justify the regressions please fix the regressions and do another perf run. If the next run shows neutral or positive results, the label will be automatically removed.

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

tmiasko · 2021-07-30T20:34:26Z

Perf comparison made while keeping partitioning fixed (by estimating size of all mono items as 1): https://perf.rust-lang.org/compare.html?start=5239e46af8c42a7c122cff505a9f44a3a5fb44c5&end=bd1ba56000b265b08adc0ce2d7a7e2f696a53214

rust-highfive assigned petrochenkov Jul 14, 2021

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

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

This comment has been minimized.

Sign in to view

rust-highfive assigned jonas-schievink and unassigned petrochenkov Jul 14, 2021

rust-highfive assigned oli-obk and unassigned jonas-schievink Jul 14, 2021

rustbot added perf-regression Performance regression. and removed S-waiting-on-perf Status: Waiting on a perf run to be completed. labels Jul 14, 2021

tmiasko force-pushed the local-copy-propagation branch from 70fa3c5 to f23d460 Compare July 14, 2021 17:11

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

bjorn3 reviewed Jul 14, 2021

View reviewed changes

compiler/rustc_mir/src/transform/copy_propagation.rs Outdated Show resolved Hide resolved

shamatar reviewed Jul 14, 2021

View reviewed changes

bjorn3 reviewed Jul 14, 2021

View reviewed changes

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

tmiasko force-pushed the local-copy-propagation branch from c322723 to ea40f9d Compare July 19, 2021 21:50

tmiasko added 5 commits July 30, 2021 14:32

Move ever_borrowed_locals to utilities

4e12b1f

Allow Operand::Copy with non-Copy types during MIR optimizations

19857d9

Local copy propagation

a40adb3

Based on ideas from 76723.

Add mir-opt test for derived PartialEq and PartialOrd

dfcb771

./x.py test --bless

bebe67e

tmiasko force-pushed the local-copy-propagation branch from ea40f9d to ba76f5e Compare July 30, 2021 12:59

This comment has been minimized.

Sign in to view

tmiasko force-pushed the local-copy-propagation branch from b7fe965 to bebe67e Compare July 30, 2021 20:31

tmiasko closed this Aug 4, 2021

tmiasko deleted the local-copy-propagation branch August 4, 2021 16:55

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Local copy propagation #87126

Local copy propagation #87126

tmiasko commented Jul 14, 2021

rust-highfive commented Jul 14, 2021

tmiasko commented Jul 14, 2021

rust-timer commented Jul 14, 2021

bors commented Jul 14, 2021

This comment has been minimized.

bors commented Jul 14, 2021

rust-timer commented Jul 14, 2021

petrochenkov commented Jul 14, 2021

jonas-schievink commented Jul 14, 2021

tmiasko commented Jul 14, 2021

rust-timer commented Jul 14, 2021

tmiasko commented Jul 14, 2021

rust-timer commented Jul 14, 2021

bors commented Jul 14, 2021

shamatar Jul 14, 2021 •

edited

Loading

tmiasko Jul 14, 2021

bjorn3 Jul 14, 2021

bors commented Jul 18, 2021

rust-timer commented Jul 18, 2021

rust-timer commented Jul 18, 2021

bors commented Jul 27, 2021

tmiasko commented Jul 30, 2021

bors commented Jul 30, 2021

This comment has been minimized.

bors commented Jul 30, 2021

tmiasko commented Jul 30, 2021

rust-timer commented Jul 30, 2021

bors commented Jul 30, 2021

bors commented Jul 30, 2021

rust-timer commented Jul 30, 2021

tmiasko commented Jul 30, 2021

rust-timer commented Jul 30, 2021

rust-timer commented Jul 30, 2021

tmiasko commented Jul 30, 2021

Local copy propagation #87126

Local copy propagation #87126

Conversation

tmiasko commented Jul 14, 2021

rust-highfive commented Jul 14, 2021

tmiasko commented Jul 14, 2021

rust-timer commented Jul 14, 2021

bors commented Jul 14, 2021

This comment has been minimized.

bors commented Jul 14, 2021

rust-timer commented Jul 14, 2021

petrochenkov commented Jul 14, 2021

jonas-schievink commented Jul 14, 2021

tmiasko commented Jul 14, 2021

rust-timer commented Jul 14, 2021

tmiasko commented Jul 14, 2021

rust-timer commented Jul 14, 2021

bors commented Jul 14, 2021

shamatar Jul 14, 2021 • edited Loading

Choose a reason for hiding this comment

tmiasko Jul 14, 2021

Choose a reason for hiding this comment

bjorn3 Jul 14, 2021

Choose a reason for hiding this comment

bors commented Jul 18, 2021

rust-timer commented Jul 18, 2021

rust-timer commented Jul 18, 2021

bors commented Jul 27, 2021

tmiasko commented Jul 30, 2021

bors commented Jul 30, 2021

This comment has been minimized.

bors commented Jul 30, 2021

tmiasko commented Jul 30, 2021

rust-timer commented Jul 30, 2021

bors commented Jul 30, 2021

bors commented Jul 30, 2021

rust-timer commented Jul 30, 2021

tmiasko commented Jul 30, 2021

rust-timer commented Jul 30, 2021

rust-timer commented Jul 30, 2021

tmiasko commented Jul 30, 2021

shamatar Jul 14, 2021 •

edited

Loading