CTFE core engine allocation & memory API improvemenets #85376

RalfJung · 2021-05-16T17:01:59Z

This is a first step towards rust-lang/miri#841.

make Allocation API offset-based (no more making up Pointers just to access an Allocation)
make Memory API higher-level (combine checking for access and getting access into one operation)

The Miri-side PR is at rust-lang/miri#1804.
r? @oli-obk

rust-highfive · 2021-05-16T17:02:01Z

Some changes occured to rustc_codegen_cranelift

cc @bjorn3

Some changes occured to the CTFE / Miri engine

cc @rust-lang/miri

Some changes occured to the CTFE / Miri engine

cc @rust-lang/miri

bjorn3 · 2021-05-16T17:03:09Z

compiler/rustc_codegen_cranelift/src/constant.rs

@@ -176,8 +176,7 @@ pub(crate) fn codegen_const_value<'tcx>(
                    std::iter::repeat(0).take(size.bytes_usize()).collect::<Vec<u8>>(),
                    align,
                );
-                let ptr = Pointer::new(AllocId(!0), Size::ZERO); // The alloc id is never used
-                alloc.write_scalar(fx, ptr, x.into(), size).unwrap();
+                alloc.write_scalar(fx, alloc_range(Size::ZERO, size), x.into()).unwrap();


Nice, no more dummy pointers!

compiler/rustc_middle/src/mir/interpret/allocation.rs

RalfJung · 2021-05-16T17:08:31Z

I am curious if this will have any perf impact... I did reduce the number of times we query the alloc_map a bit.
@bors try @rust-timer queue

rust-timer · 2021-05-16T17:08:32Z

Awaiting bors try build completion.

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

bors · 2021-05-16T17:08:41Z

⌛ Trying commit e968add0d2e7080e67498dc6f5903cd8dc0aa2de with merge de65837601ca2358d97f64015476026c73caadd6...

RalfJung · 2021-05-16T18:00:22Z

Re-try after pushing a fmt fix.
@bors try

bors · 2021-05-16T18:00:31Z

⌛ Trying commit 3f3aeca70f8e1e6873b060149a2611d371c69bf3 with merge 70a1cef598f09b48d15858b4ea6894e1df0c3f40...

bors · 2021-05-16T18:41:45Z

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

rust-timer · 2021-05-16T18:41:46Z

Queued 70a1cef598f09b48d15858b4ea6894e1df0c3f40 with parent f8e1e92, future comparison URL.

rust-timer · 2021-05-16T22:34:50Z

Finished benchmarking try commit (70a1cef598f09b48d15858b4ea6894e1df0c3f40): comparison url.

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. Please note that if the perf results are neutral, you should likely undo the rollup=never given below by specifying rollup- to bors.

Importantly, though, if the results of this run are non-neutral do not roll this PR up -- it will mask other regressions or improvements in the roll up.

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

oli-obk · 2021-05-16T23:38:54Z

so many different changes intermingled in one commit 😢 this is going to take me forever to review as right now I'm not able to be at a computer for long enough to untangle all of that.

RalfJung · 2021-05-17T07:20:01Z

I didn't really find a way to split this into separate commits that makes sense on their own (in the sense that they at least pass ./x.py check) -- but you should be able to mostly separately review the changes in librustc_middle and librustc_mir. I used to have those in separate commits but squashed them together as the individual commits wouldn't even build. It's really not "many different changes" but the two mentioned in the PR description -- which are deeply connected since after changing the Allocation API, using the old Memory API would have been grossly unsafe (in particular in terms of ensuring that we always call the access hooks).

Thinking about it, it might be possible to use the new memory API with the old allocation API (I didn't think of that since I made the changes bottom-up, i.e., allocation API first). But you should be getting basically the same effect by reviewing split-by-folders as I suggested.

RalfJung · 2021-05-17T11:53:14Z

The Miri-side PR is ready: rust-lang/miri#1804.

compiler/rustc_middle/src/mir/interpret/allocation.rs

RalfJung · 2021-05-18T07:49:00Z

Rebased.

compiler/rustc_mir/src/interpret/traits.rs

oli-obk · 2021-05-18T11:22:32Z

thanks! this was much better to review, just a nit, then r=me

- make Allocation API offset-based (no more Pointer) - make Memory API higher-level (combine checking for access and getting access into one operation)

RalfJung · 2021-05-18T17:34:33Z

Wow, that was quick, thanks :)
@bors r=oli-obk

bors · 2021-05-18T17:34:34Z

📌 Commit d5ccf68 has been approved by oli-obk

bors · 2021-05-19T10:11:33Z

⌛ Testing commit d5ccf68 with merge 3e827cc...

bors · 2021-05-19T12:52:23Z

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

rust-highfive · 2021-05-19T12:52:51Z

📣 Toolstate changed by #85376!

Tested on commit 3e827cc.
Direct link to PR: #85376

💔 miri on windows: test-pass → build-fail (cc @RalfJung @oli-obk @eddyb).
💔 miri on linux: test-pass → build-fail (cc @RalfJung @oli-obk @eddyb).

@RalfJung

Tested on commit rust-lang/rust@3e827cc. Direct link to PR: <rust-lang/rust#85376> 💔 miri on windows: test-pass → build-fail (cc @RalfJung @oli-obk @eddyb). 💔 miri on linux: test-pass → build-fail (cc @RalfJung @oli-obk @eddyb).

update for Memory API changes The Miri side of rust-lang/rust#85376.

Miri: fix alignment check in array initialization rust-lang#85376 introduced a regression in Miri, reported at rust-lang/miri#1919 and rust-lang/miri#1925. This PR fixes that. I will add tests to Miri once this lands. r? `@oli-obk` Fixes rust-lang/miri#1919 Fixes rust-lang/miri#1925

rust-highfive assigned oli-obk May 16, 2021

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

bjorn3 reviewed May 16, 2021

View reviewed changes

RalfJung commented May 16, 2021

View reviewed changes

compiler/rustc_middle/src/mir/interpret/allocation.rs Show resolved Hide resolved

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

RalfJung force-pushed the ptrless-allocs branch from e968add to 88ab68f Compare May 16, 2021 17:09

This comment has been minimized.

Sign in to view

RalfJung force-pushed the ptrless-allocs branch from 3f3aeca to 8db327f Compare May 16, 2021 22:31

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

RalfJung mentioned this pull request May 17, 2021

remove size field from Allocation #85408

Merged

This comment has been minimized.

Sign in to view

RalfJung force-pushed the ptrless-allocs branch from 59a1af7 to b767e4e Compare May 17, 2021 11:44

RalfJung mentioned this pull request May 17, 2021

update for Memory API changes rust-lang/miri#1804

Merged

RalfJung commented May 17, 2021

View reviewed changes

compiler/rustc_middle/src/mir/interpret/allocation.rs Outdated Show resolved Hide resolved

RalfJung commented May 17, 2021

View reviewed changes

compiler/rustc_middle/src/mir/interpret/allocation.rs Show resolved Hide resolved

oli-obk reviewed May 18, 2021

View reviewed changes

compiler/rustc_mir/src/interpret/traits.rs Outdated Show resolved Hide resolved

RalfJung added 4 commits May 18, 2021 19:33

CTFE core engine allocation & memory API improvemenets

46c2286

- make Allocation API offset-based (no more Pointer) - make Memory API higher-level (combine checking for access and getting access into one operation)

reduce number of allocation lookups during copy

74995c4

add Align::ONE; add methods to access alloc.extra

563ab4a

fix mplace_access_checked with forced alignment

d5ccf68

RalfJung force-pushed the ptrless-allocs branch from ebad55b to d5ccf68 Compare May 18, 2021 17:34

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 May 18, 2021

bors added the merged-by-bors This PR was explicitly merged by bors. label May 19, 2021

bors merged commit 3e827cc into rust-lang:master May 19, 2021

rustbot added this to the 1.54.0 milestone May 19, 2021

rust-highfive mentioned this pull request May 19, 2021

miri no longer builds after rust-lang/rust#85376 #85468

Closed

bors mentioned this pull request May 19, 2021

read_target_uint => read_target_ptr outside rustc_middle (nfc) #84826

Closed

RalfJung deleted the ptrless-allocs branch May 19, 2021 13:28

bors added a commit to rust-lang/miri that referenced this pull request May 19, 2021

Auto merge of #1804 - RalfJung:ptrless-allocs, r=RalfJung

06f4794

update for Memory API changes The Miri side of rust-lang/rust#85376.

bors added a commit to rust-lang/miri that referenced this pull request May 19, 2021

Auto merge of #1804 - RalfJung:ptrless-allocs, r=RalfJung

1ae39d9

update for Memory API changes The Miri side of rust-lang/rust#85376.

bors added a commit to rust-lang/miri that referenced this pull request May 19, 2021

Auto merge of #1804 - RalfJung:ptrless-allocs, r=RalfJung

b0e5d5f

update for Memory API changes The Miri side of rust-lang/rust#85376.

eddyb mentioned this pull request May 24, 2021

rustup: update to nightly-2021-05-24. EmbarkStudios/rust-gpu#631

Merged

This was referenced Nov 27, 2021

False Positive - incorrect alignment rust-lang/miri#1925

Closed

Miri: fix alignment check in array initialization #91303

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

CTFE core engine allocation & memory API improvemenets #85376

CTFE core engine allocation & memory API improvemenets #85376

RalfJung commented May 16, 2021 •

edited

Loading

rust-highfive commented May 16, 2021

bjorn3 May 16, 2021

RalfJung commented May 16, 2021

rust-timer commented May 16, 2021

bors commented May 16, 2021

This comment has been minimized.

RalfJung commented May 16, 2021

bors commented May 16, 2021

This comment has been minimized.

bors commented May 16, 2021

rust-timer commented May 16, 2021

rust-timer commented May 16, 2021

oli-obk commented May 16, 2021

RalfJung commented May 17, 2021 •

edited

Loading

This comment has been minimized.

RalfJung commented May 17, 2021

RalfJung commented May 18, 2021

oli-obk commented May 18, 2021

RalfJung commented May 18, 2021

bors commented May 18, 2021

bors commented May 19, 2021

bors commented May 19, 2021

rust-highfive commented May 19, 2021

CTFE core engine allocation & memory API improvemenets #85376

CTFE core engine allocation & memory API improvemenets #85376

Conversation

RalfJung commented May 16, 2021 • edited Loading

rust-highfive commented May 16, 2021

bjorn3 May 16, 2021

Choose a reason for hiding this comment

RalfJung commented May 16, 2021

rust-timer commented May 16, 2021

bors commented May 16, 2021

This comment has been minimized.

RalfJung commented May 16, 2021

bors commented May 16, 2021

This comment has been minimized.

bors commented May 16, 2021

rust-timer commented May 16, 2021

rust-timer commented May 16, 2021

oli-obk commented May 16, 2021

RalfJung commented May 17, 2021 • edited Loading

This comment has been minimized.

RalfJung commented May 17, 2021

RalfJung commented May 18, 2021

oli-obk commented May 18, 2021

RalfJung commented May 18, 2021

bors commented May 18, 2021

bors commented May 19, 2021

bors commented May 19, 2021

rust-highfive commented May 19, 2021

RalfJung commented May 16, 2021 •

edited

Loading

RalfJung commented May 17, 2021 •

edited

Loading