WIP: more enum layout optimizations

[mikebenfield]

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
• Contrary to my comment in Missed enum layout optimization with a NonZeroU64 + more space in an enum #101567, I only look at existing niches when creating a Flag, even though any field will do
• Keeping a Vec of 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 Flag probably makes no sense
• There probably should be some repr to force an enum to use a Flag. 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.

[rust-highfive]

r? @jackh726

(rust-highfive has picked a reviewer for you, use r? to override)

[rustbot]

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

[bors]

☔ The latest upstream changes (presumably #98588) made this pull request unmergeable. Please resolve the merge conflicts.

[mikebenfield]

• Fixed issues pointed out by @RalfJung
• Use SmallVec in a couple places
• better sorting (maybe) of niches
• better search for a niche when using a flag

[mikebenfield]

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.

[mikebenfield]

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.

[bors]

☔ The latest upstream changes (presumably #102051) made this pull request unmergeable. Please resolve the merge conflicts.

[mikebenfield]

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.

[mikebenfield]

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.

[rust-log-analyzer]

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.

[bors]

☔ The latest upstream changes (presumably #102875) made this pull request unmergeable. Please resolve the merge conflicts.

[jackh726]

Do you want a review of this or are you still working on it?

[mikebenfield]

Let's hold off on a review for now. I'm going to modify it to fix #102997 also.

[JohnCSimon]

@mikebenfield
ping from triage - can you post your status on this PR? Thanks

[mikebenfield]

@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.

[mikebenfield]

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.

[Dylan-DPC]

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