Avoid a branch on key being local for queries that use the same local and extern providers #85830

bjorn3 · 2021-05-30T15:27:55Z

Currently based on #85810 as it slightly conflicts with it. Only the last two commits are new.

rust-highfive · 2021-05-30T15:28:00Z

Some changes occured to rustc_codegen_cranelift

cc @bjorn3

rust-highfive · 2021-05-30T15:28:01Z

r? @petrochenkov

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

bjorn3 · 2021-05-30T15:28:18Z

@bors try @rust-timer queue

rust-timer · 2021-05-30T15:28:19Z

Awaiting bors try build completion.

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

bors · 2021-05-30T15:28:27Z

⌛ Trying commit 97e68edf5733b98351d0e6d43dac872661409cc0 with merge 9de0e79ac427d1d61a6e91b5c8cf50f66526d1f4...

bors · 2021-05-30T16:28:52Z

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

rust-timer · 2021-05-30T16:28:53Z

Queued 9de0e79ac427d1d61a6e91b5c8cf50f66526d1f4 with parent 2023cc3, future comparison URL.

rust-timer · 2021-05-30T18:09:41Z

Finished benchmarking try commit (9de0e79ac427d1d61a6e91b5c8cf50f66526d1f4): 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

bjorn3 · 2021-05-30T19:07:58Z

On average this is an improvement, but there is also a non-trivial amount of regressions. Maybe different inlining? Alternatively it could also be because of #85810. I will do a perf run there.

petrochenkov · 2021-05-30T19:11:37Z

r? @cjgillot

cjgillot

I am not convinced yet that removing one branch is worth the added complexity. The branch itself should be optimized out.

Can we ensure statically that Key::query_crate can only return non-LOCAL_CRATE when separate_provide_extern is set? Should we even?

cjgillot · 2021-06-08T18:08:24Z

compiler/rustc_middle/src/dep_graph/dep_node.rs

@@ -140,11 +140,11 @@ macro_rules! is_eval_always_attr {
 }

 macro_rules! contains_anon_attr {
-    ($($attr:ident $(($($attr_args:tt)*))* ),*) => ({$(is_anon_attr!($attr) | )* false});


Can all these macros be simplified now?

cjgillot · 2021-06-08T18:21:41Z

compiler/rustc_metadata/src/rmeta/decoder/cstore_impl.rs

@@ -56,7 +56,7 @@ macro_rules! provide {
                $compute
            })*

-            *providers = Providers {
+            *providers = ExternProviders {


Is this provide exhaustive now? Should it be made so?

Due to macro implementation issues I am using () as type for queries that don't have a separate extern provider.

bjorn3 · 2021-06-08T18:59:26Z

I am not convinced yet that removing one branch is worth the added complexity. The branch itself should be optimized out.

It has a side effect of for the most part documenting which queries are directly calculated from HIR and whose calculated value may be stored in the crate metadata and which are calculated in the current compilation session from other queries and configuration options.

bors · 2021-06-15T17:13:43Z

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

JohnCSimon · 2021-07-05T14:54:31Z

@rustbot label: -S-waiting-on-review +S-waiting-on-author

JohnCSimon · 2021-07-05T14:57:24Z

triage:
@bjorn3 -can you please resolve the merge conflict?

…-obk Fix bug with query modifier parsing The previous macro_rules! parsers failed when an additional modifier was added with ambiguity errors. The error is pretty unclear as to what exactly the cause here is, but this change simplifies the argument parsing code such that the error is avoided. Extracted from other work, and somewhat duplicates 0358edeb5 from rust-lang#85830, but this approach seems a little simpler to me. Not technically currently necessary but seems like a good cleanup.

camelid · 2021-10-08T00:44:11Z

triage: What's the status of this PR?

JohnCSimon · 2021-10-24T21:51:30Z

Ping from triage:
@bjorn3
What is the status of this PR?

… and extern providers

bjorn3 · 2021-10-25T11:37:24Z

Rebased. This is waiting on a decision if it should be merged in the first place.

cjgillot · 2021-10-25T19:10:12Z

After consideration, I think this PR is an opportunity for an eventual project to integrate metadata and queries.
@bors r+

bors · 2021-10-25T19:10:13Z

📌 Commit 13abc1a has been approved by cjgillot

bors · 2021-10-26T00:39:01Z

⌛ Testing commit 13abc1a with merge 17e13b5...

bors · 2021-10-26T03:42:20Z

☀️ Test successful - checks-actions
Approved by: cjgillot
Pushing 17e13b5 to master...

rust-highfive · 2021-10-26T03:42:46Z

📣 Toolstate changed by #85830!

Tested on commit 17e13b5.
Direct link to PR: #85830

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

@RalfJung

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

rust-timer · 2021-10-26T05:14:17Z

Finished benchmarking commit (17e13b5): comparison url.

Summary: This benchmark run did not return any relevant changes.

If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf.

@rustbot label: -perf-regression

rustup Rustup for rust-lang/rust#85830

bjorn3 added T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. A-query-system Area: The rustc query system (https://rustc-dev-guide.rust-lang.org/query.html) labels May 30, 2021

rust-highfive assigned petrochenkov May 30, 2021

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

bjorn3 force-pushed the separate_provide_extern branch from 3e94f3b to 97e68ed Compare May 30, 2021 15:28

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

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

rust-highfive assigned cjgillot and unassigned petrochenkov May 30, 2021

bjorn3 force-pushed the separate_provide_extern branch from 97e68ed to 664b109 Compare June 8, 2021 17:27

This comment has been minimized.

Sign in to view

cjgillot reviewed Jun 8, 2021

View reviewed changes

bjorn3 mentioned this pull request Jun 11, 2021

Preserve more spans in internal rustc_queries! macro #86123

Merged

rustbot 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-review Status: Awaiting review from the assignee but also interested parties. labels Jul 5, 2021

JohnCSimon added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Sep 20, 2021

cjgillot mentioned this pull request Sep 29, 2021

Refactor fingerprint reconstruction #89343

Merged

Mark-Simulacrum mentioned this pull request Oct 2, 2021

Fix bug with query modifier parsing #89466

Merged

Avoid a branch on key being local for queries that use the same local…

f5c3e83

… and extern providers

bjorn3 force-pushed the separate_provide_extern branch from 32e1948 to f5c3e83 Compare October 25, 2021 11:36

This comment has been minimized.

Sign in to view

Fix test

13abc1a

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 Oct 25, 2021

bors added the merged-by-bors This PR was explicitly merged by bors. label Oct 26, 2021

bors merged commit 17e13b5 into rust-lang:master Oct 26, 2021

rustbot added this to the 1.58.0 milestone Oct 26, 2021

rust-highfive mentioned this pull request Oct 26, 2021

miri no longer builds after rust-lang/rust#85830 #90298

Closed

bjorn3 deleted the separate_provide_extern branch October 26, 2021 08:48

bjorn3 mentioned this pull request Oct 26, 2021

rustup rust-lang/miri#1903

Merged

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

Auto merge of #1903 - bjorn3:rustup, r=oli-obk

04e5c78

rustup Rustup for rust-lang/rust#85830

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Avoid a branch on key being local for queries that use the same local and extern providers #85830

Avoid a branch on key being local for queries that use the same local and extern providers #85830

bjorn3 commented May 30, 2021

rust-highfive commented May 30, 2021

rust-highfive commented May 30, 2021

bjorn3 commented May 30, 2021

rust-timer commented May 30, 2021

bors commented May 30, 2021

bors commented May 30, 2021

rust-timer commented May 30, 2021

rust-timer commented May 30, 2021

bjorn3 commented May 30, 2021 •

edited

Loading

petrochenkov commented May 30, 2021

This comment has been minimized.

cjgillot left a comment

cjgillot Jun 8, 2021

cjgillot Jun 8, 2021

bjorn3 Jun 8, 2021

bjorn3 commented Jun 8, 2021

bors commented Jun 15, 2021

JohnCSimon commented Jul 5, 2021

JohnCSimon commented Jul 5, 2021

camelid commented Oct 8, 2021

JohnCSimon commented Oct 24, 2021

bjorn3 commented Oct 25, 2021

This comment has been minimized.

cjgillot commented Oct 25, 2021

bors commented Oct 25, 2021

bors commented Oct 26, 2021

bors commented Oct 26, 2021

rust-highfive commented Oct 26, 2021

rust-timer commented Oct 26, 2021

Avoid a branch on key being local for queries that use the same local and extern providers #85830

Avoid a branch on key being local for queries that use the same local and extern providers #85830

Conversation

bjorn3 commented May 30, 2021

rust-highfive commented May 30, 2021

rust-highfive commented May 30, 2021

bjorn3 commented May 30, 2021

rust-timer commented May 30, 2021

bors commented May 30, 2021

bors commented May 30, 2021

rust-timer commented May 30, 2021

rust-timer commented May 30, 2021

bjorn3 commented May 30, 2021 • edited Loading

petrochenkov commented May 30, 2021

This comment has been minimized.

cjgillot left a comment

Choose a reason for hiding this comment

cjgillot Jun 8, 2021

Choose a reason for hiding this comment

cjgillot Jun 8, 2021

Choose a reason for hiding this comment

bjorn3 Jun 8, 2021

Choose a reason for hiding this comment

bjorn3 commented Jun 8, 2021

bors commented Jun 15, 2021

JohnCSimon commented Jul 5, 2021

JohnCSimon commented Jul 5, 2021

camelid commented Oct 8, 2021

JohnCSimon commented Oct 24, 2021

bjorn3 commented Oct 25, 2021

This comment has been minimized.

cjgillot commented Oct 25, 2021

bors commented Oct 25, 2021

bors commented Oct 26, 2021

bors commented Oct 26, 2021

rust-highfive commented Oct 26, 2021

rust-timer commented Oct 26, 2021

bjorn3 commented May 30, 2021 •

edited

Loading