Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Add a hook for should_codegen_locally #127779

Merged
merged 3 commits into from
Jul 20, 2024
Merged

Conversation

momvart
Copy link
Contributor

@momvart momvart commented Jul 15, 2024

This PR lifts the module-local function should_codegen_locally to TyCtxt as a hook.
In addition to monomorphization, this function is used for checking the dependency of compiler_builtins on other libraries. Moving this function to the hooks also makes overriding it possible for the tools that use the rustc interface.

@rustbot
Copy link
Collaborator

rustbot commented Jul 15, 2024

r? @cjgillot

rustbot has assigned @cjgillot.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jul 15, 2024
@rust-log-analyzer

This comment has been minimized.

@momvart momvart force-pushed the should_codegen_hook branch from 34fc498 to d3cc29a Compare July 15, 2024 22:09
@rust-log-analyzer

This comment has been minimized.

@momvart momvart force-pushed the should_codegen_hook branch from d3cc29a to 745fe57 Compare July 15, 2024 23:13
@rustbot
Copy link
Collaborator

rustbot commented Jul 15, 2024

Some changes occurred in compiler/rustc_codegen_cranelift

cc @bjorn3

@rust-log-analyzer

This comment has been minimized.

@momvart momvart changed the title Add a hook for should_codegen_locally Draft: Add a hook for should_codegen_locally Jul 15, 2024
@momvart momvart changed the title Draft: Add a hook for should_codegen_locally Add a hook for should_codegen_locally Jul 15, 2024
@momvart momvart marked this pull request as draft July 15, 2024 23:43
@momvart momvart force-pushed the should_codegen_hook branch from 745fe57 to 9b80250 Compare July 15, 2024 23:44
@momvart momvart marked this pull request as ready for review July 16, 2024 00:56
@cjgillot
Copy link
Contributor

@bors try @rust-timer queue

@rust-timer

This comment has been minimized.

@rustbot rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Jul 19, 2024
bors added a commit to rust-lang-ci/rust that referenced this pull request Jul 19, 2024
Add a hook for `should_codegen_locally`

This PR lifts the module-local function `should_codegen_locally` to `TyCtxt` as a hook.
In addition to monomorphization, this function is used for checking the dependency of `compiler_builtins` on other libraries. Moving this function to the hooks also makes overriding it possible for the tools that use the rustc interface.
@bors
Copy link
Contributor

bors commented Jul 19, 2024

⌛ Trying commit 9b80250 with merge 071bac7...

@bors
Copy link
Contributor

bors commented Jul 20, 2024

☀️ Try build successful - checks-actions
Build commit: 071bac7 (071bac7a19aed5905c989a2c50c2660145ee8869)

@rust-timer

This comment has been minimized.

@rust-timer
Copy link
Collaborator

Finished benchmarking commit (071bac7): comparison URL.

Overall result: ❌ regressions - no action needed

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. While you can manually mark this PR as fit for rollup, we strongly recommend not doing so since this PR may lead to changes in compiler perf.

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

Instruction count

This is a highly reliable metric that was used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
0.3% [0.3%, 0.3%] 1
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) - - 0

Max RSS (memory usage)

This benchmark run did not return any relevant results for this metric.

Cycles

This benchmark run did not return any relevant results for this metric.

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 767.644s -> 770.693s (0.40%)
Artifact size: 328.87 MiB -> 328.85 MiB (-0.01%)

@rustbot rustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Jul 20, 2024
@cjgillot
Copy link
Contributor

@bors r+ rollup-

@bors
Copy link
Contributor

bors commented Jul 20, 2024

📌 Commit 9b80250 has been approved by cjgillot

It is now in the queue for this repository.

@bors bors removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 20, 2024
@bors bors added the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Jul 20, 2024
tgross35 added a commit to tgross35/rust that referenced this pull request Jul 20, 2024
…illot

Add a hook for `should_codegen_locally`

This PR lifts the module-local function `should_codegen_locally` to `TyCtxt` as a hook.
In addition to monomorphization, this function is used for checking the dependency of `compiler_builtins` on other libraries. Moving this function to the hooks also makes overriding it possible for the tools that use the rustc interface.
tgross35 added a commit to tgross35/rust that referenced this pull request Jul 20, 2024
…illot

Add a hook for `should_codegen_locally`

This PR lifts the module-local function `should_codegen_locally` to `TyCtxt` as a hook.
In addition to monomorphization, this function is used for checking the dependency of `compiler_builtins` on other libraries. Moving this function to the hooks also makes overriding it possible for the tools that use the rustc interface.
bors added a commit to rust-lang-ci/rust that referenced this pull request Jul 20, 2024
…iaskrgr

Rollup of 6 pull requests

Successful merges:

 - rust-lang#127463 ( use precompiled rustdoc with CI rustc)
 - rust-lang#127779 (Add a hook for `should_codegen_locally`)
 - rust-lang#127843 (unix: document unsafety for std `sig{action,altstack}`)
 - rust-lang#127873 (kmc-solid: `#![forbid(unsafe_op_in_unsafe_fn)]`)
 - rust-lang#127917 (match lowering: Split `finalize_or_candidate` into more coherent methods)
 - rust-lang#127964 (run_make_support: skip rustfmt for lib.rs)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 9a6f8cc into rust-lang:master Jul 20, 2024
7 checks passed
@rustbot rustbot added this to the 1.81.0 milestone Jul 20, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Jul 20, 2024
Rollup merge of rust-lang#127779 - momvart:should_codegen_hook, r=cjgillot

Add a hook for `should_codegen_locally`

This PR lifts the module-local function `should_codegen_locally` to `TyCtxt` as a hook.
In addition to monomorphization, this function is used for checking the dependency of `compiler_builtins` on other libraries. Moving this function to the hooks also makes overriding it possible for the tools that use the rustc interface.
bjorn3 pushed a commit to bjorn3/rust that referenced this pull request Aug 9, 2024
…illot

Add a hook for `should_codegen_locally`

This PR lifts the module-local function `should_codegen_locally` to `TyCtxt` as a hook.
In addition to monomorphization, this function is used for checking the dependency of `compiler_builtins` on other libraries. Moving this function to the hooks also makes overriding it possible for the tools that use the rustc interface.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants