Skip to content

Store THIR in IndexVecs instead of an Arena #83842

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Jump to bottom
Merged
merged 2 commits into from
May 19, 2021
Merged

Store THIR in IndexVecs instead of an Arena #83842

merged 2 commits into from
May 19, 2021

Conversation

LeSeulArtichaut
Copy link
Contributor

@LeSeulArtichaut LeSeulArtichaut commented Apr 4, 2021 •
edited
Loading

This is a necessary step to store the THIR in a query: #85273. See relevant discussion on Zulip.

r? @ghost cc @cjgillot @nikomatsakis

@rust-log-analyzer

This comment has been minimized.

# to view
@cjgillot
Copy link
Contributor

cjgillot commented Apr 4, 2021

@bors try @rust-timer queue

@rust-timer
Copy link
Collaborator

Awaiting bors try build completion.

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

@rustbot rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Apr 4, 2021
@bors
Copy link
Collaborator

bors commented Apr 4, 2021

⌛ Trying commit 222ac61592ccac5b91f8a3a16c3f385a1c6cb23e with merge d782db89e0c40c3406d6e9785d86798f2fb6419d...

@LeSeulArtichaut LeSeulArtichaut added the S-experimental Status: Ongoing experiment that does not require reviewing and won't be merged in its current state. label Apr 4, 2021
@cjgillot
Copy link
Contributor

cjgillot commented Apr 4, 2021

@bors try @rust-timer queue

@rust-timer
Copy link
Collaborator

Awaiting bors try build completion.

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

@bors
Copy link
Collaborator

bors commented Apr 4, 2021

⌛ Trying commit b760ebdd0512856ff5806a3984a75ee1a0bb60fd with merge 33f4e8ad3bd066cbf2e0ef92800759e35434b3c3...

@bors
Copy link
Collaborator

bors commented Apr 4, 2021

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

@rust-timer
Copy link
Collaborator

Queued 33f4e8ad3bd066cbf2e0ef92800759e35434b3c3 with parent 88e7862, future comparison URL.

@rust-timer
Copy link
Collaborator

Finished benchmarking try commit (33f4e8ad3bd066cbf2e0ef92800759e35434b3c3): 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

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-perf Status: Waiting on a perf run to be completed. labels Apr 4, 2021
@bors

This comment has been minimized.

# to view
@rust-log-analyzer

This comment has been minimized.

# to view
@rust-log-analyzer

This comment has been minimized.

# to view
@rust-log-analyzer

This comment has been minimized.

# to view
@rust-log-analyzer

This comment has been minimized.

# to view
@bors

This comment has been minimized.

# to view
@LeSeulArtichaut
Copy link
Contributor Author

@Mark-Simulacrum The perf impact hasn't been mitigated since the perf run you mentioned, but this allows us to create a stealable query for THIR (#85273). I've added a bit of context in the PR description.

@LeSeulArtichaut
Copy link
Contributor Author

@bors r=nikomatsakis rollup=never

@bors
Copy link
Collaborator

bors commented May 19, 2021

📌 Commit 7093a21 has been approved by nikomatsakis

@bors 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 19, 2021
@bors
Copy link
Collaborator

bors commented May 19, 2021

⌛ Testing commit 7093a21 with merge f2a2183241cdacc66f12cc7abdbd714550066537...

@bors
Copy link
Collaborator

bors commented May 19, 2021

💔 Test failed - checks-actions

@bors 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 May 19, 2021
@rust-log-analyzer
Copy link
Collaborator

A job failed! Check out the build log: (web) (plain)

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

@lqd
Copy link
Member

lqd commented May 19, 2021

Do we expect making a stealable query for THIR to alleviate the max-rss increase seen in the perf run above btw ?

@lqd
Copy link
Member

lqd commented May 19, 2021

@bors retry

looks like an intermittent failure, other PRs have had a networking issue on the same mac builder

@bors 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 19, 2021
@bors
Copy link
Collaborator

bors commented May 19, 2021

⌛ Testing commit 7093a21 with merge f94942d...

@bors
Copy link
Collaborator

bors commented May 19, 2021

☀️ Test successful - checks-actions
Approved by: nikomatsakis
Pushing f94942d to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label May 19, 2021
@bors bors merged commit f94942d into rust-lang:master May 19, 2021
@rustbot rustbot added this to the 1.54.0 milestone May 19, 2021
This was referenced May 19, 2021
Merged
Merged
@LeSeulArtichaut LeSeulArtichaut deleted the thir-vec branch May 19, 2021 21:53
@matthiaskrgr matthiaskrgr mentioned this pull request May 21, 2021
Closed
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request May 21, 2021
@GuillaumeGomez
Rollup merge of rust-lang#85306 - LeSeulArtichaut:thir-unsafeck, r=ni…
88f5744 
…komatsakis

Add DerefOfRawPointer and CallToFunctionWith to THIR unsafeck

Extends THIR unsafeck to check for two more cases of unsafe operations: dereferences of raw pointers and calls to functions with `#[target_feature]` (RFC 2396). The check for the latter is pretty much copy-pasted from the existing MIR equivalent.

This will clash with rust-lang#83842 and rust-lang#85273 which are arguably more important, let's maybe focus on getting those merged first, this can wait.
r? `@nikomatsakis`
cc rust-lang/project-thir-unsafeck#7
bors added a commit to rust-lang-ci/rust that referenced this pull request May 25, 2021
@bors
Auto merge of rust-lang#85273 - LeSeulArtichaut:thir-query, r=nikomat…
d568d63 
…sakis

Make building THIR a stealable query

This PR creates a stealable `thir_body` query so that we can build the THIR only once for THIR unsafeck and MIR build.

Blocked on rust-lang#83842.
r? `@nikomatsakis`
# for free to join this conversation on GitHub. Already have an account? # to comment
Reviewers

@lqd lqd lqd left review comments

@cjgillot cjgillot cjgillot left review comments

@nikomatsakis nikomatsakis nikomatsakis approved these changes

Assignees

@nikomatsakis nikomatsakis

Labels
merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Milestone
1.54.0
Development

Successfully merging this pull request may close these issues.
10 participants
@LeSeulArtichaut @rust-log-analyzer @cjgillot @rust-timer @bors @jonas-schievink @nikomatsakis @Mark-Simulacrum @lqd @rustbot