rust
rust copied to clipboard
rust-lang
WIP: more enum layout optimizations
Work in progress, but it works.
In addition to addressing #101567, also keep around multiple niches rather than just the largest one, and for tagged variants possibly move fields around the niche rather than just putting the whole variant before or after.
Things that aren't right at the moment:
- [ ] I haven't fixed cranelift codegen
- [ ] I'm just emitting bogus debuginfo in
rustc_codegen_llvm/src/debuginfo/metadata/enums/mod.rs - [X] Contrary to my comment in #101567, I only look at existing niches when creating a
Flag, even though any field will do - [ ] Keeping a
Vecof niches rather than just the largest one likely has a significant negative effect on performance, but I've done no performance testing - [ ] Comments should be updated/added to explain what's going on with
Flag - [ ] The current sorting of niches in the presence of
Flagprobably makes no sense - [ ] There probably should be some
reprto force anenumto use aFlag. Right now an enum like the one given in the issue:
pub enum Provenance {
Concrete {
alloc_id: NonZeroU64,
sb: NonZeroU64,
},
Wildcard,
}
will just use a regular niche, which means Option<Provenance> won't be able to use a flag.
r? @jackh726
(rust-highfive has picked a reviewer for you, use r? to override)
Some changes occurred in compiler/rustc_codegen_cranelift
cc @bjorn3
Some changes occurred to the CTFE / Miri engine
cc @rust-lang/miri
Some changes occurred in src/librustdoc/clean/types.rs
cc @camelid
The job mingw-check failed! Check out the build log: (web) (plain)
Click to see the possible cause of the failure (guessed by this bot)
Successfully built 24466cae975a
Successfully tagged rust-ci:latest
Built container sha256:24466cae975af64876ac757dbf31ebf19f47edd8075ff919005aa2f8eab5b3a0
Uploading finished image to https://ci-caches.rust-lang.org/docker/100443de95ac5fd5d76f947e37204434fbf3cf3da99f888ec8bb4fbbceda7ed7d93b519998f8388225735159bb6cbee56e76ad72dc699a1cf736f3af809f03d0
upload failed: - to s3://rust-lang-ci-sccache2/docker/100443de95ac5fd5d76f947e37204434fbf3cf3da99f888ec8bb4fbbceda7ed7d93b519998f8388225735159bb6cbee56e76ad72dc699a1cf736f3af809f03d0 Unable to locate credentials
[CI_JOB_NAME=mingw-check]
---
Checking rustc_codegen_cranelift v0.1.0 (/checkout/compiler/rustc_codegen_cranelift)
error[E0027]: pattern does not mention field `flag`
--> src/discriminant.rs:45:27
|
45 | tag_encoding: TagEncoding::Niche { untagged_variant, ref niche_variants, niche_start },
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ missing field `flag`
help: include the missing field in the pattern
|
|
45 | tag_encoding: TagEncoding::Niche { untagged_variant, ref niche_variants, niche_start, flag },
| ~~~~~~~~
help: if you don't care about this missing field, you can explicitly ignore it
|
45 | tag_encoding: TagEncoding::Niche { untagged_variant, ref niche_variants, niche_start, .. },
error[E0027]: pattern does not mention field `flag`
--> src/discriminant.rs:116:9
|
|
116 | TagEncoding::Niche { untagged_variant, ref niche_variants, niche_start } => {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ missing field `flag`
help: include the missing field in the pattern
|
|
116 | TagEncoding::Niche { untagged_variant, ref niche_variants, niche_start, flag } => {
| ~~~~~~~~
help: if you don't care about this missing field, you can explicitly ignore it
|
116 | TagEncoding::Niche { untagged_variant, ref niche_variants, niche_start, .. } => {
For more information about this error, try `rustc --explain E0027`.
error: could not compile `rustc_codegen_cranelift` due to 2 previous errors
Build completed unsuccessfully in 0:03:21
:umbrella: The latest upstream changes (presumably #98588) made this pull request unmergeable. Please resolve the merge conflicts.
The job mingw-check failed! Check out the build log: (web) (plain)
Click to see the possible cause of the failure (guessed by this bot)
Checking rustc_codegen_cranelift v0.1.0 (/checkout/compiler/rustc_codegen_cranelift)
error[E0027]: pattern does not mention field `flag`
--> src/discriminant.rs:45:27
|
45 | tag_encoding: TagEncoding::Niche { untagged_variant, ref niche_variants, niche_start },
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ missing field `flag`
help: include the missing field in the pattern
|
|
45 | tag_encoding: TagEncoding::Niche { untagged_variant, ref niche_variants, niche_start, flag },
| ~~~~~~~~
help: if you don't care about this missing field, you can explicitly ignore it
|
45 | tag_encoding: TagEncoding::Niche { untagged_variant, ref niche_variants, niche_start, .. },
error[E0027]: pattern does not mention field `flag`
--> src/discriminant.rs:116:9
|
|
116 | TagEncoding::Niche { untagged_variant, ref niche_variants, niche_start } => {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ missing field `flag`
help: include the missing field in the pattern
|
|
116 | TagEncoding::Niche { untagged_variant, ref niche_variants, niche_start, flag } => {
| ~~~~~~~~
help: if you don't care about this missing field, you can explicitly ignore it
|
116 | TagEncoding::Niche { untagged_variant, ref niche_variants, niche_start, .. } => {
For more information about this error, try `rustc --explain E0027`.
error: could not compile `rustc_codegen_cranelift` due to 2 previous errors
Build completed unsuccessfully in 0:02:51
- Fixed issues pointed out by @RalfJung
- Use
SmallVecin a couple places - better sorting (maybe) of niches
- better search for a niche when using a flag
The job mingw-check failed! Check out the build log: (web) (plain)
Click to see the possible cause of the failure (guessed by this bot)
Checking rustc_codegen_cranelift v0.1.0 (/checkout/compiler/rustc_codegen_cranelift)
error[E0027]: pattern does not mention field `flag`
--> src/discriminant.rs:45:27
|
45 | tag_encoding: TagEncoding::Niche { untagged_variant, ref niche_variants, niche_start },
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ missing field `flag`
help: include the missing field in the pattern
|
|
45 | tag_encoding: TagEncoding::Niche { untagged_variant, ref niche_variants, niche_start, flag },
| ~~~~~~~~
help: if you don't care about this missing field, you can explicitly ignore it
|
45 | tag_encoding: TagEncoding::Niche { untagged_variant, ref niche_variants, niche_start, .. },
error[E0027]: pattern does not mention field `flag`
--> src/discriminant.rs:116:9
|
|
116 | TagEncoding::Niche { untagged_variant, ref niche_variants, niche_start } => {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ missing field `flag`
help: include the missing field in the pattern
|
|
116 | TagEncoding::Niche { untagged_variant, ref niche_variants, niche_start, flag } => {
| ~~~~~~~~
help: if you don't care about this missing field, you can explicitly ignore it
|
116 | TagEncoding::Niche { untagged_variant, ref niche_variants, niche_start, .. } => {
For more information about this error, try `rustc --explain E0027`.
error: could not compile `rustc_codegen_cranelift` due to 2 previous errors
Build completed unsuccessfully in 0:02:47
I implemented repr(flag), which can go on an enum to indicate that we should try a niche with a new flag first, rather than after looking at existing niches. I also added Option<Provenance> from #101567 as a test case.
About repr(flag):
- its name probably should change;
- I'm not sure it should exist;
- I assume it should go through an RFC process or something.
The job mingw-check failed! Check out the build log: (web) (plain)
Click to see the possible cause of the failure (guessed by this bot)
Checking rustc_codegen_cranelift v0.1.0 (/checkout/compiler/rustc_codegen_cranelift)
error[E0027]: pattern does not mention field `flag`
--> src/discriminant.rs:45:27
|
45 | tag_encoding: TagEncoding::Niche { untagged_variant, ref niche_variants, niche_start },
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ missing field `flag`
help: include the missing field in the pattern
|
|
45 | tag_encoding: TagEncoding::Niche { untagged_variant, ref niche_variants, niche_start, flag },
| ~~~~~~~~
help: if you don't care about this missing field, you can explicitly ignore it
|
45 | tag_encoding: TagEncoding::Niche { untagged_variant, ref niche_variants, niche_start, .. },
error[E0027]: pattern does not mention field `flag`
--> src/discriminant.rs:116:9
|
|
116 | TagEncoding::Niche { untagged_variant, ref niche_variants, niche_start } => {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ missing field `flag`
help: include the missing field in the pattern
|
|
116 | TagEncoding::Niche { untagged_variant, ref niche_variants, niche_start, flag } => {
| ~~~~~~~~
help: if you don't care about this missing field, you can explicitly ignore it
|
116 | TagEncoding::Niche { untagged_variant, ref niche_variants, niche_start, .. } => {
For more information about this error, try `rustc --explain E0027`.
error: could not compile `rustc_codegen_cranelift` due to 2 previous errors
Build completed unsuccessfully in 0:02:49
The job mingw-check failed! Check out the build log: (web) (plain)
Click to see the possible cause of the failure (guessed by this bot)
Checking rustc_codegen_cranelift v0.1.0 (/checkout/compiler/rustc_codegen_cranelift)
error[E0027]: pattern does not mention field `flag`
--> src/discriminant.rs:45:27
|
45 | tag_encoding: TagEncoding::Niche { untagged_variant, ref niche_variants, niche_start },
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ missing field `flag`
help: include the missing field in the pattern
|
|
45 | tag_encoding: TagEncoding::Niche { untagged_variant, ref niche_variants, niche_start, flag },
| ~~~~~~~~
help: if you don't care about this missing field, you can explicitly ignore it
|
45 | tag_encoding: TagEncoding::Niche { untagged_variant, ref niche_variants, niche_start, .. },
error[E0027]: pattern does not mention field `flag`
--> src/discriminant.rs:116:9
|
|
116 | TagEncoding::Niche { untagged_variant, ref niche_variants, niche_start } => {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ missing field `flag`
help: include the missing field in the pattern
|
|
116 | TagEncoding::Niche { untagged_variant, ref niche_variants, niche_start, flag } => {
| ~~~~~~~~
help: if you don't care about this missing field, you can explicitly ignore it
|
116 | TagEncoding::Niche { untagged_variant, ref niche_variants, niche_start, .. } => {
For more information about this error, try `rustc --explain E0027`.
error: could not compile `rustc_codegen_cranelift` due to 2 previous errors
Build completed unsuccessfully in 0:03:30
:umbrella: The latest upstream changes (presumably #102051) made this pull request unmergeable. Please resolve the merge conflicts.
Status: In working on this, I've encountered what I'm 95% sure is a bug in the LLVM InstCombine pass that manifests in some cases with these layout optimizations. I'm going to investigate more, and then either fix that or report it, then will return to this PR.
The job mingw-check failed! Check out the build log: (web) (plain)
Click to see the possible cause of the failure (guessed by this bot)
Checking rustc_codegen_cranelift v0.1.0 (/checkout/compiler/rustc_codegen_cranelift)
error[E0027]: pattern does not mention field `flag`
--> src/discriminant.rs:45:27
|
45 | tag_encoding: TagEncoding::Niche { untagged_variant, ref niche_variants, niche_start },
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ missing field `flag`
help: include the missing field in the pattern
|
|
45 | tag_encoding: TagEncoding::Niche { untagged_variant, ref niche_variants, niche_start, flag },
| ~~~~~~~~
help: if you don't care about this missing field, you can explicitly ignore it
|
45 | tag_encoding: TagEncoding::Niche { untagged_variant, ref niche_variants, niche_start, .. },
error[E0027]: pattern does not mention field `flag`
--> src/discriminant.rs:116:9
|
|
116 | TagEncoding::Niche { untagged_variant, ref niche_variants, niche_start } => {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ missing field `flag`
help: include the missing field in the pattern
|
|
116 | TagEncoding::Niche { untagged_variant, ref niche_variants, niche_start, flag } => {
| ~~~~~~~~
help: if you don't care about this missing field, you can explicitly ignore it
|
116 | TagEncoding::Niche { untagged_variant, ref niche_variants, niche_start, .. } => {
For more information about this error, try `rustc --explain E0027`.
error: could not compile `rustc_codegen_cranelift` due to 2 previous errors
Build completed unsuccessfully in 0:04:01
Rebased. Since some things were moved around it was most expedient to just squash the commits.
I fixed the problem I mentioned above, which was in fact not an LLVM bug. The issue is that when there's a flag, the values of the niche are only actually restricted to its valid_range when the flag is set to its magic value. The solution I've adopted for now is, when there's a flag, to just emit the full range of the primitive as the valid range (see rustc_middle/src/ty/layout.rs). An alternative would be, when there's a flag, only access the niche when the flag is set to magic_value. This would require a couple extra branches in computing the discriminant, although maybe LLVM could optimize those branches away. I'll experiment at some point.
The job mingw-check failed! Check out the build log: (web) (plain)
Click to see the possible cause of the failure (guessed by this bot)
configure: rust.debug-assertions := True
configure: rust.overflow-checks := True
configure: llvm.assertions := True
configure: dist.missing-tools := True
configure: build.configure-args := ['--enable-sccache', '--disable-manage-submodu ...
configure: writing `config.toml` in current directory
configure:
configure: run `python /checkout/x.py --help`
Attempting with retry: make prepare
---
skip untracked path cpu-usage.csv during rustfmt invocations
skip untracked path src/doc/book/ during rustfmt invocations
skip untracked path src/doc/rust-by-example/ during rustfmt invocations
skip untracked path src/llvm-project/ during rustfmt invocations
Diff in /checkout/compiler/rustc_codegen_cranelift/src/abi/comments.rs at line 82:
}
}
let TyAndLayout { ty, layout } = place.layout();
- let rustc_target::abi::LayoutS {
- align,
- abi: _,
- variants: _,
- fields: _,
- fields: _,
- niches: _,
- } = layout.0.0;
+ let rustc_target::abi::LayoutS { size, align, abi: _, variants: _, fields: _, niches: _ } =
+ layout.0.0;
let (kind, extra) = match *place.inner() {
CPlaceInner::Var(place_local, var) => {
Diff in /checkout/compiler/rustc_codegen_cranelift/src/discriminant.rs at line 42:
Variants::Multiple {
tag: _,
tag_field,
- tag_encoding: TagEncoding::Niche { untagged_variant, ref niche_variants, niche_start, .. },
+ tag_encoding:
+ TagEncoding::Niche { untagged_variant, ref niche_variants, niche_start, .. },
variants: _,
} => {
if variant_index != untagged_variant {
Running `"/checkout/obj/build/x86_64-unknown-linux-gnu/stage0/bin/rustfmt" "--config-path" "/checkout" "--edition" "2021" "--unstable-features" "--skip-children" "--check" "/checkout/compiler/rustc_codegen_cranelift/src/abi/comments.rs" "/checkout/compiler/rustc_codegen_cranelift/src/vtable.rs" "/checkout/compiler/rustc_codegen_cranelift/src/lib.rs" "/checkout/compiler/rustc_codegen_cranelift/src/abi/returning.rs" "/checkout/compiler/rustc_codegen_cranelift/src/abi/mod.rs" "/checkout/compiler/rustc_codegen_cranelift/src/abi/pass_mode.rs" "/checkout/compiler/rustc_codegen_cranelift/src/optimize/peephole.rs" "/checkout/compiler/rustc_codegen_cranelift/src/main_shim.rs"` failed.
If you're running `tidy`, try again with `--bless`. Or, if you just want to format code, run `./x.py fmt` instead.
The job mingw-check failed! Check out the build log: (web) (plain)
Click to see the possible cause of the failure (guessed by this bot)
configure: rust.debug-assertions := True
configure: rust.overflow-checks := True
configure: llvm.assertions := True
configure: dist.missing-tools := True
configure: build.configure-args := ['--enable-sccache', '--disable-manage-submodu ...
configure: writing `config.toml` in current directory
configure:
configure: run `python /checkout/x.py --help`
Attempting with retry: make prepare
---
skip untracked path src/doc/rust-by-example/ during rustfmt invocations
skip untracked path src/llvm-project/ during rustfmt invocations
Diff in /checkout/compiler/rustc_target/src/abi/mod.rs at line 1195:
) -> Option<Self> {
let niche_data = |offset, scalar| {
let Scalar::Initialized {value, valid_range } = scalar else { return None };
- Some(NicheData {
- value,
- valid_range,
- })
- })
+ Some(NicheData { offset, value, valid_range })
let niche = Niche {
Diff in /checkout/compiler/rustc_target/src/abi/mod.rs at line 1206:
data: niche_data(niche_offset, niche_scalar)?,
- flag: Some(
- niche_data(flag_offset, flag_scalar)?,
- )
+ flag: Some(niche_data(flag_offset, flag_scalar)?),
if niche.available(cx) > 0 || niche.available_with_new_flag(cx) > 0 {
Diff in /checkout/compiler/rustc_target/src/abi/mod.rs at line 1237:
let niche_size = self.data.value.size(cx);
assert!(niche_size.bits() <= 128);
let max = size.unsigned_int_max();
- if max < u128::MAX {
- max + 1
- max
- }
- }
+ if max < u128::MAX { max + 1 } else { max }
0
}
Diff in /checkout/compiler/rustc_target/src/abi/mod.rs at line 1248:
})
})
}
- pub fn reserve_with_new_flag<C: HasDataLayout>(&self, cx: &C, count: u128) -> Option<(u128, Scalar, Scalar)> {
+ pub fn reserve_with_new_flag<C: HasDataLayout>(
+ &self,
+ cx: &C,
+ count: u128,
+ ) -> Option<(u128, Scalar, Scalar)> {
assert!(count > 0);
self.flag.map_or(None, |flag_data| {
Running `"/checkout/obj/build/x86_64-unknown-linux-gnu/stage0/bin/rustfmt" "--config-path" "/checkout" "--edition" "2021" "--unstable-features" "--skip-children" "--check" "/checkout/compiler/rustc_ast/src/token.rs" "/checkout/compiler/rustc_llvm/src/lib.rs" "/checkout/compiler/rustc_llvm/build.rs" "/checkout/compiler/rustc_ast/src/util/comments.rs" "/checkout/compiler/rustc_traits/src/lib.rs" "/checkout/compiler/rustc_target/src/json.rs" "/checkout/compiler/rustc_target/src/abi/mod.rs" "/checkout/compiler/rustc_ast/src/entry.rs"` failed.
If you're running `tidy`, try again with `--bless`. Or, if you just want to format code, run `./x.py fmt` instead.
:umbrella: The latest upstream changes (presumably #102875) made this pull request unmergeable. Please resolve the merge conflicts.
Let's hold off on a review for now. I'm going to modify it to fix #102997 also.
@mikebenfield ping from triage - can you post your status on this PR? Thanks
@JohnCSimon I've got local in-progress changes that are semi-close to being done. I've been meaning to get back to this anyway so I'll see if I can get them done and pushed here today or tomorrow. Can't guarantee it'll be done though.
Status: I think I'll have this ready for review by end of week.
I had it pretty much done and then got laid off and lost access to the code I had written 😂 . So I kind of had to start from scratch, but it should get done quickly this time.
Closing this as inactive. Feel free to reöpen this pr or create a new pr if you get the time to work on this. Thanks
