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

Stop using to_hir_binop in codegen #125399

Merged
merged 1 commit into from
May 22, 2024
Merged

Conversation

scottmcm
Copy link
Member

This came up in #125359 (comment) , and looking into it we can just use the mir::BinOps directly instead of hir::BinOpKinds.

(AKA rather than going mir::BinOphir::BinOpKindIntPredicate, just go mir::BinOpIntPredicate.)

@rustbot
Copy link
Collaborator

rustbot commented May 22, 2024

r? @wesleywiser

rustbot has assigned @wesleywiser.
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 22, 2024
@RalfJung
Copy link
Member

So where is that function even still used now? Maybe we can just get rid of it entirely...

@rust-log-analyzer

This comment has been minimized.

@scottmcm scottmcm force-pushed the less-hir-in-cg_ssa branch from e779b14 to ccd1136 Compare May 22, 2024 08:17
@scottmcm
Copy link
Member Author

It's used here

Overflow(binop, left, right) => {
add!("op", binop.to_hir_binop().as_str());
add!("left", format!("{left:#?}"));
add!("right", format!("{right:#?}"));
}

and

Expr::Binop(op, c1, c2) => {
let precedence = |binop: rustc_middle::mir::BinOp| {
use rustc_ast::util::parser::AssocOp;
AssocOp::from_ast_binop(binop.to_hir_binop().into()).precedence()
};
let op_precedence = precedence(op);
let formatted_op = op.to_hir_binop().as_str();

They're not as trivially-removable, but they're both in rustc_middle so I made it pub(crate) to at least discourage more uses.

@rust-log-analyzer

This comment has been minimized.

@scottmcm scottmcm force-pushed the less-hir-in-cg_ssa branch from ccd1136 to 8ee3d29 Compare May 22, 2024 08:34
@rustbot
Copy link
Collaborator

rustbot commented May 22, 2024

Some changes occurred in compiler/rustc_codegen_gcc

cc @antoyo, @GuillaumeGomez

@compiler-errors
Copy link
Member

r? compiler-errors @bors r+ rollup

@bors
Copy link
Contributor

bors commented May 22, 2024

📌 Commit 8ee3d29 has been approved by compiler-errors

It is now in the queue for this repository.

@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 22, 2024
fmease added a commit to fmease/rust that referenced this pull request May 22, 2024
…piler-errors

Stop using `to_hir_binop` in codegen

This came up in rust-lang#125359 (comment) , and looking into it we can just use the `mir::BinOp`s directly instead of `hir::BinOpKind`s.

(AKA rather than going `mir::BinOp` → `hir::BinOpKind` → `IntPredicate`, just go `mir::BinOp` → `IntPredicate`.)
bors added a commit to rust-lang-ci/rust that referenced this pull request May 22, 2024
Rollup of 7 pull requests

Successful merges:

 - rust-lang#125043 (reference type safety invariant docs: clarification)
 - rust-lang#125306 (Force the inner coroutine of an async closure to `move` if the outer closure is `move` and `FnOnce`)
 - rust-lang#125355 (Use Backtrace::force_capture instead of Backtrace::capture in rustc_log)
 - rust-lang#125378 (remove tracing tree indent lines)
 - rust-lang#125391 (Minor serialize/span tweaks)
 - rust-lang#125395 (Remove unnecessary `.md` from the documentation sidebar)
 - rust-lang#125399 (Stop using `to_hir_binop` in codegen)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this pull request May 22, 2024
Rollup of 7 pull requests

Successful merges:

 - rust-lang#125043 (reference type safety invariant docs: clarification)
 - rust-lang#125306 (Force the inner coroutine of an async closure to `move` if the outer closure is `move` and `FnOnce`)
 - rust-lang#125355 (Use Backtrace::force_capture instead of Backtrace::capture in rustc_log)
 - rust-lang#125382 (rustdoc: rename `issue-\d+.rs` tests to have meaningful names (part 7))
 - rust-lang#125391 (Minor serialize/span tweaks)
 - rust-lang#125395 (Remove unnecessary `.md` from the documentation sidebar)
 - rust-lang#125399 (Stop using `to_hir_binop` in codegen)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this pull request May 22, 2024
Rollup of 7 pull requests

Successful merges:

 - rust-lang#125043 (reference type safety invariant docs: clarification)
 - rust-lang#125306 (Force the inner coroutine of an async closure to `move` if the outer closure is `move` and `FnOnce`)
 - rust-lang#125355 (Use Backtrace::force_capture instead of Backtrace::capture in rustc_log)
 - rust-lang#125382 (rustdoc: rename `issue-\d+.rs` tests to have meaningful names (part 7))
 - rust-lang#125391 (Minor serialize/span tweaks)
 - rust-lang#125395 (Remove unnecessary `.md` from the documentation sidebar)
 - rust-lang#125399 (Stop using `to_hir_binop` in codegen)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 412c46c into rust-lang:master May 22, 2024
6 checks passed
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request May 22, 2024
Rollup merge of rust-lang#125399 - scottmcm:less-hir-in-cg_ssa, r=compiler-errors

Stop using `to_hir_binop` in codegen

This came up in rust-lang#125359 (comment) , and looking into it we can just use the `mir::BinOp`s directly instead of `hir::BinOpKind`s.

(AKA rather than going `mir::BinOp` → `hir::BinOpKind` → `IntPredicate`, just go `mir::BinOp` → `IntPredicate`.)
@rustbot rustbot added this to the 1.80.0 milestone May 22, 2024
@scottmcm scottmcm deleted the less-hir-in-cg_ssa branch May 23, 2024 04:34
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.

7 participants