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

Normalize consts in writeback when GCE is enabled #130645

Merged
merged 2 commits into from
Sep 21, 2024

Conversation

compiler-errors
Copy link
Member

GCE lazily normalizes its unevaluated consts. This PR ensures that, like the new solver with its lazy norm types, we can assume that the writeback results are fully normalized.

This is important since we're trying to eliminate unnecessary calls to ty::Const::{eval,normalize} since they won't work with mGCE. Previously, we'd keep those consts unnormalized in writeback all the way through MIR build, and they'd only get normalized if we explicitly called ty::Const::{eval,normalize}, or during codegen since that calls normalize_erasing_regions (which invokes the QueryNormalizer, which evaluates the const accordingly).

This hack can (hopefully obviously) be removed when mGCE is implemented and we yeet the old GCE; it's only reachable with the GCE flag anyways, so I'm not worried about the implications here.

r? @BoxyUwU

@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 Sep 21, 2024
@BoxyUwU
Copy link
Member

BoxyUwU commented Sep 21, 2024

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Sep 21, 2024

📌 Commit 920a973 has been approved by BoxyUwU

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 Sep 21, 2024
bors added a commit to rust-lang-ci/rust that referenced this pull request Sep 21, 2024
…iaskrgr

Rollup of 6 pull requests

Successful merges:

 - rust-lang#129718 (add guarantee about remove_dir and remove_file error kinds)
 - rust-lang#130598 (Add recursion limit to FFI safety lint)
 - rust-lang#130642 (Pass the current cargo to `run-make` tests)
 - rust-lang#130644 (Only expect valtree consts in codegen)
 - rust-lang#130645 (Normalize consts in writeback when GCE is enabled)
 - rust-lang#130646 (compiler: factor out `OVERFLOWING_LITERALS` impl)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit e6cf3bd into rust-lang:master Sep 21, 2024
6 checks passed
@rustbot rustbot added this to the 1.83.0 milestone Sep 21, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Sep 21, 2024
Rollup merge of rust-lang#130645 - compiler-errors:normalize-gce-writeback, r=BoxyUwU

Normalize consts in writeback when GCE is enabled

GCE lazily normalizes its unevaluated consts. This PR ensures that, like the new solver with its lazy norm types, we can assume that the writeback results are fully normalized.

This is important since we're trying to eliminate unnecessary calls to `ty::Const::{eval,normalize}` since they won't work with mGCE. Previously, we'd keep those consts unnormalized in writeback all the way through MIR build, and they'd only get normalized if we explicitly called `ty::Const::{eval,normalize}`, or during codegen since that calls `normalize_erasing_regions` (which invokes the `QueryNormalizer`, which evaluates the const accordingly).

This hack can (hopefully obviously) be removed when mGCE is implemented and we yeet the old GCE; it's only reachable with the GCE flag anyways, so I'm not worried about the implications here.

r? `@BoxyUwU`
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Sep 22, 2024
…al, r=BoxyUwU

Replace calls to `ty::Const::{try_}eval` in mir build/pattern analysis

We normalize consts in writeback: rust-lang#130645. This means that consts are gonna be as normalized as they're ever gonna get in MIR building and pattern analysis. Therefore we can just use `try_to_target_usize` rather than calling `eval_target_usize`.

Regarding the `.expect` calls, I'm not totally certain whether they're correct given rigid unevaluated consts. But this PR shouldn't make *more* ICEs occur; we may have to squash these ICEs when mGCE comes around, tho 😺

r? `@BoxyUwU`
workingjubilee added a commit to workingjubilee/rustc that referenced this pull request Sep 23, 2024
…al, r=BoxyUwU

Replace calls to `ty::Const::{try_}eval` in mir build/pattern analysis

We normalize consts in writeback: rust-lang#130645. This means that consts are gonna be as normalized as they're ever gonna get in MIR building and pattern analysis. Therefore we can just use `try_to_target_usize` rather than calling `eval_target_usize`.

Regarding the `.expect` calls, I'm not totally certain whether they're correct given rigid unevaluated consts. But this PR shouldn't make *more* ICEs occur; we may have to squash these ICEs when mGCE comes around, tho 😺
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Sep 23, 2024
…al, r=BoxyUwU

Replace calls to `ty::Const::{try_}eval` in mir build/pattern analysis

We normalize consts in writeback: rust-lang#130645. This means that consts are gonna be as normalized as they're ever gonna get in MIR building and pattern analysis. Therefore we can just use `try_to_target_usize` rather than calling `eval_target_usize`.

Regarding the `.expect` calls, I'm not totally certain whether they're correct given rigid unevaluated consts. But this PR shouldn't make *more* ICEs occur; we may have to squash these ICEs when mGCE comes around, tho 😺
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Sep 23, 2024
Rollup merge of rust-lang#130715 - compiler-errors:mir-build-const-eval, r=BoxyUwU

Replace calls to `ty::Const::{try_}eval` in mir build/pattern analysis

We normalize consts in writeback: rust-lang#130645. This means that consts are gonna be as normalized as they're ever gonna get in MIR building and pattern analysis. Therefore we can just use `try_to_target_usize` rather than calling `eval_target_usize`.

Regarding the `.expect` calls, I'm not totally certain whether they're correct given rigid unevaluated consts. But this PR shouldn't make *more* ICEs occur; we may have to squash these ICEs when mGCE comes around, tho 😺
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.

4 participants