Remove redundant information in rustc_abi::Variants#151742
Remove redundant information in rustc_abi::Variants#151742moulins wants to merge 6 commits intorust-lang:mainfrom
rustc_abi::Variants#151742Conversation
For now, this is a 1-to-1 copy of `LayoutData`, but this will change.
It was always set to `Variants::Single`.
Enum variants always have `Arbitrary` layout, so the enum isn't needed.
This field is only used during layout calculations, so re-synthetized `LayoutData`s for enum variants can use a dummy value instead.
Reusing the alignment of the enclosing enum in `LayoutData::for_variant` doesn't appear to cause any issues.
The only place inspecting the niches of a variant layout is `compiler/rustc_codegen_llvm/src/debuginfo/metadata/enums/mod.rs`, which can be rewritten to use the niches of the enclosing enum instead.
|
r? @wesleywiser rustbot has assigned @wesleywiser. Use |
| #[derive(PartialEq, Eq, Hash, Clone, Debug)] | ||
| #[cfg_attr(feature = "nightly", derive(HashStable_Generic))] | ||
| pub struct VariantLayout<FieldIdx: Idx> { | ||
| pub size: Size, |
There was a problem hiding this comment.
Can't be removed, as it is used by the variant_size_differences lint.
| #[cfg_attr(feature = "nightly", derive(HashStable_Generic))] | ||
| pub struct VariantLayout<FieldIdx: Idx> { | ||
| pub size: Size, | ||
| pub backend_repr: BackendRepr, |
There was a problem hiding this comment.
I don't know enough about codegen to confidently say if it requires accurate reprs for enum variants, so I've left this field for now.
| pub size: Size, | ||
| pub backend_repr: BackendRepr, | ||
| pub field_offsets: IndexVec<FieldIdx, Size>, | ||
| fields_in_memory_order: IndexVec<u32, FieldIdx>, |
There was a problem hiding this comment.
Note: this field could be removed and recomputed from the field offsets in Layout::from_variant; it's unclear whether this is worth it.
| // Remove discriminant values of the other variants from the largest niche. This assumes | ||
| // that the largest niche, when it exists, always corresponds to the enum discriminant. |
There was a problem hiding this comment.
This assumption was already implicitly made by the original code.
| let min = valid_range.start.min(valid_range.end); | ||
| let min = tag.size(cx).truncate(min); | ||
|
|
||
| let max = valid_range.start.max(valid_range.end); | ||
| let max = tag.size(cx).truncate(max); |
There was a problem hiding this comment.
I think this logic is broken for valid ranges wrapping around uN::MAX? In any case, fixing this is out-of-scope of the PR, so I've left it as-is.
|
@bors try @rust-timer queue |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Remove redundant information in `rustc_abi::Variants`
This comment has been minimized.
This comment has been minimized.
|
Finished benchmarking commit (968e542): comparison URL. Overall result: ✅ improvements - no action neededBenchmarking this pull request means it may be perf-sensitive – we'll automatically label it not fit for rolling up. You can override this, but we strongly advise not to, due to possible changes in compiler perf. @bors rollup=never Instruction countOur most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.
Max RSS (memory usage)Results (primary 1.4%, secondary -2.8%)A less reliable metric. May be of interest, but not used to determine the overall result above.
CyclesResults (secondary 3.7%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Binary sizeResults (primary 0.0%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Bootstrap: 470.891s -> 473.88s (0.63%) |
Follow-up to #151040; partially addresses #113988.
Replaces the nested
LayoutDatainVariants::Multipleby a new, smallerVariantLayoutstruct, and adjustLayoutData::for_variantand the layout algorithm in consequence.This PR is best reviewed commit-by-commit.