Skip to content

miri: use AllocId instead of u64. - #47205

Merged
bors merged 1 commit into
rust-lang:masterfrom
eddyb:alloc-id
Jan 6, 2018
Merged

miri: use AllocId instead of u64.#47205
bors merged 1 commit into
rust-lang:masterfrom
eddyb:alloc-id

Conversation

@eddyb

@eddybeddyb commented Jan 5, 2018

Copy link
Copy Markdown
Contributor

This makes @alexreg's miri allocation -> LLVM global translation more straight-forward.

r? @oli-obk

@kennytmkennytm added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jan 5, 2018
@oli-obk

oli-obk commented Jan 5, 2018

Copy link
Copy Markdown
Contributor

Ha. That was on my list. Thanks for doing it. Lgtm

@eddyb

eddyb commented Jan 5, 2018

Copy link
Copy Markdown
ContributorAuthor

@bors delegate=oli-obk

@bors

bors commented Jan 5, 2018

Copy link
Copy Markdown
Collaborator

✌️ @oli-obk can now approve this pull request

oli-obk
oli-obk approved these changes Jan 5, 2018
View reviewed changes

@oli-obkoli-obk left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's readd the pointer once we implement the optimization

@bors r+

Comment threadsrc/librustc/ty/context.rs
///
/// The pointers are to the beginning of an `alloc_by_id` allocation
alloc_cache: FxHashMap<interpret::GlobalId<'tcx>, interpret::Pointer>,
alloc_cache: FxHashMap<interpret::GlobalId<'tcx>, interpret::AllocId>,

@oli-obkoli-obkJan 5, 2018

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The idea was that you could have globals which are located inside other locals in the future. So if a constant's value is obtained by indexing another constant, we'd just point to the offset in the array constant.

@eddybeddybJan 5, 2018

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We do need miri allocations properly exposed for miri alloc -> LLVM global translation, or at least some guarantee that statics don't get optimized like that.
FWIW I think MemoryPointer would be a better fit, eventually.

@oli-obk

oli-obk commented Jan 5, 2018

Copy link
Copy Markdown
Contributor

@bors r+

Maybe bors doesn't like reviews?

@bors

bors commented Jan 5, 2018

Copy link
Copy Markdown
Collaborator

📌 Commit 0907494 has been approved by oli-obk

@kennytmkennytm 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 Jan 6, 2018
kennytm added a commit to kennytm/rust that referenced this pull request Jan 6, 2018
@kennytm
Rollup merge ofrust-lang#47205- eddyb:alloc-id, r=oli-obk
c6bf11c
miri: use AllocId instead of u64.
This makes @alexreg's miri allocation -> LLVM global translation more straight-forward.
r? @oli-obk
@kennytmkennytm mentioned this pull request Jan 6, 2018
Merged
bors added a commit that referenced this pull request Jan 6, 2018
@bors
Auto merge of#47235- kennytm:rollup, r=kennytm
6828cf9
Rollup of 7 pull requests
- Successful merges: #46947, #47170, #47190, #47205, #47217, #47220, #47230
- Failed merges: #47233
@bors
bors merged commit 0907494 into rust-lang:masterJan 6, 2018
@eddyb
eddyb deleted the alloc-id branch January 6, 2018 23:34
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@oli-obkoli-obkoli-obk approved these changes

Assignees

No one assigned

Labels

S-waiting-on-borsStatus: Waiting on bors to run and complete tests. Bors will change the label on completion.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@eddyb@oli-obk@bors@kennytm