rust-lang / rust-lang/rust

Missed enum layout optimization when Enum contains primitive with multiple niches and other variant have small data

Open
#160,054 3 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

A-enum A-layout C-optimization F-pattern_types needs-triage T-compiler
Dominant language
Rust
Stars
119k
Forks
16.1k
PR merge metrics
PR metrics pending

Description

I found this in the compiler - enum UnwindAction is 8 bytes, even though it could be 4, because it contains BasicBlock which has 256 niches. Other variant has a small enum to distinguish "terminate reason", but there should still be a plenty of space to put it there, because it only has two values.

Reduced it down to this:

#![feature(pattern_type_macro)]
#![feature(pattern_types)]

macro_rules! static_assert_size {
    ($ty:ty, $size:expr) => {
        const _: [(); $size] = [(); ::std::mem::size_of::<$ty>()];
    };
}

pub enum Reason {
    Abi,
    InCleanup
}

pub struct BasicBlock(pattern_type!(u32 is 0 ..= 0xFFFF_FF00));

pub enum Unwind {
    Terminate(Reason),
    Cleanup(BasicBlock),
}


static_assert_size!(Unwind, 4); // assert fails, the size is 8 currently

pub fn main(){}

I looked for similar issues on missed niche optimizations but I believe I haven't found analogous one - other issues usually contain multiple types with niches (like two NonZero* fields) and require combining types to compute discriminant, which is a more complicated variant of this optimization.


Note: as I'm writing thign, I realize that the compiler would have to pick Reason enum values based on the layout of Unwind, which we can't do at the moment (IIUC layout of a type can't depend on other type). Am I right? This is super confusing to reason about.

Either way, this is a red herring - even if you pick Reason values manually to allow Unwind to pick a place to put the discriminant and Reason values optimally, the compiler still won't do it.

Using these values for should allow the optimization (and Unwind discriminat would be computed by clearing the last bit):

pub enum Reason {
    Abi = 0x10,
    InCleanup =  0x11
}

Correct me if I'm wrong, this is really confusing to reason about.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with the reduced Rust reproducer and its failing static_assert_size!(Unwind, 4). Trace the compiler's enum layout and niche-selection handling; done means the shown Unwind case is laid out in 4 bytes and the reproducer passes.

Written by the indexing model from the issue text.

Assessment

Tech stack
rust
Domain
compilers
Issue type
Bug
Difficulty
5/5
Estimated time
Over a week
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
45/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.