Preserve unused pointer to address casts #97597

tmiasko · 2022-05-31T19:38:56Z

Fixes #97421.

cc @RalfJung

rust-highfive · 2022-05-31T19:38:58Z

Some changes occured to rustc_codegen_cranelift

cc @bjorn3

Some changes occurred in src/tools/clippy.

cc @rust-lang/clippy

Some changes occured to the CTFE / Miri engine

cc @rust-lang/miri

rust-highfive · 2022-05-31T19:39:00Z

r? @cjgillot

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

RalfJung · 2022-06-01T14:37:04Z

compiler/rustc_middle/src/mir/mod.rs

+            // <https://www.ralfj.de/blog/2022/04/11/provenance-exposed.html>
+            Rvalue::Cast(CastKind::PointerAddress, _, _) => true,
+
+            Rvalue::Use(_)


Use can potentially mutate memory if it uses an Operand::Move (which is proposed to de-initialize the memory it reads from).
Removing a Move with unused result is still okay (the optimized program simply has more defined memory than the unoptimized program), but e.g. reordering it with other reads/writes is not if they might overlap. So depending on what the caller cares about, Move needs to be considered effectful.

RalfJung · 2022-06-01T14:38:06Z

compiler/rustc_middle/src/mir/mod.rs

    #[inline]
-    pub fn is_pointer_int_cast(&self) -> bool {
-        matches!(self, Rvalue::Cast(CastKind::PointerAddress, _, _))
+    pub fn may_have_side_effects(&self) -> bool {


Please define precisely which side-effects this is talking about. Rust code has a lot of possible effects that can prevent reordering and/or treating code as 'dead' and removing it:

Mutating memory (or other machine state)

Reading from memory (or other mutable machine state)

panicking

looping forever

causing UB

making a non-deterministic choice

The easiest to define condition is whether code is entirely "pure", which means it does none of these things. Pure stuff can be reordered arbitrarily, hoisted out of conditionals/loops, removed if the result is unused -- all the good stuff. But then we have to exclude many operation (including e.g. division). The second easiest is "pure or UB", but that already lets you do fewer optimizations.

RalfJung · 2022-06-01T14:45:59Z

compiler/rustc_middle/src/mir/mod.rs

+            | Rvalue::ThreadLocalRef(_)
+            | Rvalue::AddressOf(_, _)
+            | Rvalue::Len(_)
+            | Rvalue::Cast(CastKind::Misc | CastKind::Pointer(_), _, _)


Int-to-ptr casts should be considered as making a non-deterministic choice (angelically) and reading mutable machine state (namely the set of exposed provenances), which can prevent them from being reordered with other operations.

Though I assume reads from mutable machine state are fine here since otherwise Use would also be out entirely. So my guess is the property you care about here is "can this rvalue be removed if we don't care about the result of the computation"? That would be "no mutating, no panicking, no looping forever" (and also "no angelic choice unless we can prove there exists a possible choice", but the angelic choice in int-to-ptr casts always has a trivially possible choice if the created pointer is unused so that is fine).

RalfJung · 2022-06-01T17:26:28Z

Thanks, LGTM. :)

(But I can't tell if the use sites make sense since I am not familiar with those MIR transformations.)

bors · 2022-06-02T01:42:59Z

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

cjgillot · 2022-06-02T17:12:42Z

r? @RalfJung

RalfJung · 2022-06-02T19:00:35Z

I'm happy with the 'general' part of this PR, but cannot say if the specific changes in 3 optimizations/analyses are correct, or if they need a different notion of 'no side-effect'.

The changes in liveness and DSE are obviously not changing behavior (so at least I am sure they are not making anything less correct than it was before), but I have no idea what the super_statement call in 'simplify' is about.
@rust-lang/wg-mir-opt hopefully one of you can complete the review. :)

JakobDegen · 2022-06-03T23:26:12Z

Fwiw lgtm too, but no r+ so will still need someone else to review

tmiasko · 2022-06-07T14:47:35Z

Thanks for review @RalfJung, @JakobDegen.

@bors r=RalfJung,JakobDegen

bors · 2022-06-07T14:47:36Z

📌 Commit d140d05 has been approved by RalfJung,JakobDegen

…s, r=RalfJung,JakobDegen Preserve unused pointer to address casts Fixes rust-lang#97421. cc `@RalfJung`

Dylan-DPC · 2022-06-07T15:24:42Z

failed in rollup ci

@bors r-

tmiasko · 2022-06-07T16:15:02Z

Rebased and updated match pattern to cover recently added cast kind.

@bors r=RalfJung,JakobDegen

bors · 2022-06-07T16:15:04Z

📌 Commit 6277c3a has been approved by RalfJung,JakobDegen

Rollup of 5 pull requests Successful merges: - rust-lang#97595 (Remove unwrap from get_vtable) - rust-lang#97597 (Preserve unused pointer to address casts) - rust-lang#97819 (Recover `import` instead of `use` in item) - rust-lang#97823 (Recover missing comma after match arm) - rust-lang#97851 (Use repr(C) when depending on struct layout in ptr tests) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup

rust-highfive assigned cjgillot May 31, 2022

rustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label May 31, 2022

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

tmiasko force-pushed the simplify-locals-side-effects branch from b6ddebf to 63f6324 Compare June 1, 2022 05:07

RalfJung reviewed Jun 1, 2022

View reviewed changes

tmiasko force-pushed the simplify-locals-side-effects branch from 63f6324 to 62f9c2d Compare June 1, 2022 17:15

tmiasko force-pushed the simplify-locals-side-effects branch from 62f9c2d to d140d05 Compare June 2, 2022 11:09

rust-highfive assigned RalfJung and unassigned cjgillot Jun 2, 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 Jun 7, 2022

Dylan-DPC added a commit to Dylan-DPC/rust that referenced this pull request Jun 7, 2022

Rollup merge of rust-lang#97597 - tmiasko:simplify-locals-side-effect…

82513ea

…s, r=RalfJung,JakobDegen Preserve unused pointer to address casts Fixes rust-lang#97421. cc `@RalfJung`

Dylan-DPC mentioned this pull request Jun 7, 2022

Rollup of 5 pull requests #97834

Closed

bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Jun 7, 2022

Preserve unused pointer to address casts

6277c3a

tmiasko force-pushed the simplify-locals-side-effects branch from d140d05 to 6277c3a Compare June 7, 2022 15:33

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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jun 7, 2022

Dylan-DPC mentioned this pull request Jun 8, 2022

Rollup of 5 pull requests #97860

Merged

bors merged commit d380b45 into rust-lang:master Jun 8, 2022

rustbot added this to the 1.63.0 milestone Jun 8, 2022

tmiasko deleted the simplify-locals-side-effects branch June 10, 2022 09:45

dtolnay mentioned this pull request Sep 7, 2024

ICE: symbol-mangling-version=v0 attempt to subtract with overflow #130011

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Preserve unused pointer to address casts #97597

Preserve unused pointer to address casts #97597

tmiasko commented May 31, 2022 •

edited

Loading

rust-highfive commented May 31, 2022

rust-highfive commented May 31, 2022

RalfJung Jun 1, 2022 •

edited

Loading

RalfJung Jun 1, 2022 •

edited

Loading

RalfJung Jun 1, 2022 •

edited

Loading

RalfJung commented Jun 1, 2022 •

edited

Loading

bors commented Jun 2, 2022

cjgillot commented Jun 2, 2022

RalfJung commented Jun 2, 2022 •

edited

Loading

JakobDegen commented Jun 3, 2022 •

edited

Loading

tmiasko commented Jun 7, 2022

bors commented Jun 7, 2022

Dylan-DPC commented Jun 7, 2022 •

edited

Loading

tmiasko commented Jun 7, 2022

bors commented Jun 7, 2022

Preserve unused pointer to address casts #97597

Preserve unused pointer to address casts #97597

Conversation

tmiasko commented May 31, 2022 • edited Loading

rust-highfive commented May 31, 2022

rust-highfive commented May 31, 2022

RalfJung Jun 1, 2022 • edited Loading

Choose a reason for hiding this comment

RalfJung Jun 1, 2022 • edited Loading

Choose a reason for hiding this comment

RalfJung Jun 1, 2022 • edited Loading

Choose a reason for hiding this comment

RalfJung commented Jun 1, 2022 • edited Loading

bors commented Jun 2, 2022

cjgillot commented Jun 2, 2022

RalfJung commented Jun 2, 2022 • edited Loading

JakobDegen commented Jun 3, 2022 • edited Loading

tmiasko commented Jun 7, 2022

bors commented Jun 7, 2022

Dylan-DPC commented Jun 7, 2022 • edited Loading

tmiasko commented Jun 7, 2022

bors commented Jun 7, 2022

tmiasko commented May 31, 2022 •

edited

Loading

RalfJung Jun 1, 2022 •

edited

Loading

RalfJung Jun 1, 2022 •

edited

Loading

RalfJung Jun 1, 2022 •

edited

Loading

RalfJung commented Jun 1, 2022 •

edited

Loading

RalfJung commented Jun 2, 2022 •

edited

Loading

JakobDegen commented Jun 3, 2022 •

edited

Loading

Dylan-DPC commented Jun 7, 2022 •

edited

Loading