Skip to content

A variety of improvements to the codegen backends #141448

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 9 commits into from
May 30, 2025
Merged

A variety of improvements to the codegen backends #141448

merged 9 commits into from
May 30, 2025

Conversation

bjorn3
Copy link
Member

@bjorn3 bjorn3 commented May 23, 2025

Some are just general improvements to cg_ssa or cg_llvm, while others will make it slightly easier to use cg_ssa in cg_clif in the future.

@rustbot
Copy link
Collaborator

rustbot commented May 23, 2025

r? @jieyouxu

rustbot has assigned @jieyouxu.
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 May 23, 2025
@rustbot
Copy link
Collaborator

rustbot commented May 23, 2025

Some changes occurred in compiler/rustc_codegen_gcc

cc @antoyo, @GuillaumeGomez

Some changes occurred in coverage instrumentation.

cc @Zalathar

Some changes occurred in compiler/rustc_codegen_ssa

cc @WaffleLapkin

Zalathar
Zalathar reviewed May 23, 2025
View reviewed changes
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@jieyouxu
Copy link
Member

r? compiler (a bit backed up atm)

@rustbot rustbot assigned Nadrieril and unassigned jieyouxu May 23, 2025
@bjorn3 bjorn3 force-pushed the codegen_refactors branch from 9cfd7f5 to 735486a Compare May 23, 2025 15:20
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@bjorn3 bjorn3 force-pushed the codegen_refactors branch 2 times, most recently from 2a23e5c to da3350f Compare May 27, 2025 11:27
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@Nadrieril
Copy link
Member

r? compiler dunno much about these parts

@rustbot rustbot assigned lcnr and unassigned Nadrieril May 27, 2025
@bjorn3 bjorn3 force-pushed the codegen_refactors branch from da3350f to 39bdeea Compare May 28, 2025 08:24
@rustbot rustbot added the A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. label May 28, 2025
lcnr
lcnr reviewed May 28, 2025
View reviewed changes
@bjorn3 bjorn3 force-pushed the codegen_refactors branch from 39bdeea to 47ff228 Compare May 28, 2025 09:15
@lcnr
Copy link
Contributor

lcnr commented May 28, 2025

I've looked through this while having no idea about codegen. All the changes look reasonable and mostly feel like positive changes. Can't rly judge that though

r? llvm

@rustbot
Copy link
Collaborator

rustbot commented May 28, 2025

Failed to set assignee to llvm: cannot assign: response: {"message":"Not Found","documentation_url":"https://docs.github.com/rest/issues/assignees#add-assignees-to-an-issue","status":"404"}

Note: Only org members with at least the repository "read" role, users with write permissions, or people who have commented on the PR may be assigned.

@lcnr
Copy link
Contributor

lcnr commented May 28, 2025

r? compiler

@rustbot rustbot assigned davidtwco and unassigned lcnr May 28, 2025
bjorn3 added 6 commits May 28, 2025 20:55
@bjorn3
Remove a couple of uses of interior mutability around statics
0fd257d
@bjorn3
Move supports_parallel from CodegenBackend to ExtraBackendMethods
0809b41 
It is only relevant when using cg_ssa for driving compilation.
@bjorn3
Make predefine methods take &mut self
669e2ea
@bjorn3
Remove methods from StaticCodegenMethods that are not called in cg_ss…
d7c0bde 
…a itself
@bjorn3
Mark all optimize methods and the codegen method as safe
f0707fa 
There is no safety contract and I don't think any of them can actually
cause UB in more ways than passing malicious source code to rustc can.
While LtoModuleCodegen::optimize says that the returned ModuleCodegen
points into the LTO module, the LTO module has already been dropped by
the time this function returns, so if the returned ModuleCodegen indeed
points into the LTO module, we would have seen crashes on every LTO
compilation, which we don't. As such the comment is outdated.
@bjorn3
Remove unused arg_memory_ty method
865c7b9
@bjorn3 bjorn3 force-pushed the codegen_refactors branch from 47ff228 to 865c7b9 Compare May 28, 2025 20:55
@WaffleLapkin
Copy link
Member

r? WaffleLapkin
@bors r+

@bors
Copy link
Collaborator

bors commented May 28, 2025

📌 Commit 865c7b9 has been approved by WaffleLapkin

It is now in the queue for this repository.

@rustbot rustbot assigned WaffleLapkin and unassigned davidtwco May 28, 2025
@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 28, 2025
@GuillaumeGomez GuillaumeGomez mentioned this pull request May 29, 2025
Merged
bors added a commit that referenced this pull request May 29, 2025
@bors
Auto merge of #141739 - GuillaumeGomez:rollup-ivboqwd, r=GuillaumeGomez
de6db15 
Rollup of 11 pull requests

Successful merges:

 - #137574 (Make `std/src/num` mirror `core/src/num`)
 - #141384 (Enable review queue tracking)
 - #141448 (A variety of improvements to the codegen backends)
 - #141636 (avoid some usages of `&mut P<T>` in AST visitors)
 - #141676 (float: Disable `total_cmp` sNaN tests for `f16`)
 - #141705 (Add eslint as part of `tidy` run)
 - #141715 (Add `loongarch64` with `d` feature to `f32::midpoint` fast path)
 - #141723 (Provide secrets to try builds with new bors)
 - #141728 (Fix false documentation of FnCtxt::diverges)
 - #141729 (resolve target-libdir directly from rustc)
 - #141732 (creader: Remove extraenous String::clone)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit that referenced this pull request May 29, 2025
@bors
Auto merge of #141739 - GuillaumeGomez:rollup-ivboqwd, r=GuillaumeGomez
1ac1950 
Rollup of 11 pull requests

Successful merges:

 - #137574 (Make `std/src/num` mirror `core/src/num`)
 - #141384 (Enable review queue tracking)
 - #141448 (A variety of improvements to the codegen backends)
 - #141636 (avoid some usages of `&mut P<T>` in AST visitors)
 - #141676 (float: Disable `total_cmp` sNaN tests for `f16`)
 - #141705 (Add eslint as part of `tidy` run)
 - #141715 (Add `loongarch64` with `d` feature to `f32::midpoint` fast path)
 - #141723 (Provide secrets to try builds with new bors)
 - #141728 (Fix false documentation of FnCtxt::diverges)
 - #141729 (resolve target-libdir directly from rustc)
 - #141732 (creader: Remove extraenous String::clone)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 161cf3e into rust-lang:master May 30, 2025
9 checks passed
@rustbot rustbot added this to the 1.89.0 milestone May 30, 2025
rust-timer added a commit that referenced this pull request May 30, 2025
@rust-timer
Unrolled build for #141448
6816a17 
Rollup merge of #141448 - bjorn3:codegen_refactors, r=WaffleLapkin

A variety of improvements to the codegen backends

Some are just general improvements to cg_ssa or cg_llvm, while others will make it slightly easier to use cg_ssa in cg_clif in the future.
@bjorn3 bjorn3 deleted the codegen_refactors branch May 30, 2025 08:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Zalathar Zalathar Zalathar left review comments

@lcnr lcnr lcnr left review comments

@WaffleLapkin WaffleLapkin WaffleLapkin approved these changes

Assignees

@WaffleLapkin WaffleLapkin

Labels
A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. 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
Milestone
1.89.0
Development

Successfully merging this pull request may close these issues.
10 participants
@bjorn3 @rustbot @rust-log-analyzer @jieyouxu @Nadrieril @lcnr @WaffleLapkin @bors @Zalathar @davidtwco