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.

Sign up for GitHub

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

Improve niche placement by trying two strategies and picking the better result #108106

Merged
merged 8 commits into from
Apr 29, 2023
Merged

Improve niche placement by trying two strategies and picking the better result #108106

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
be8e67d
refactor: extract function
the8472 Feb 16, 2023
faf2da3
try two different niche-placement strategies when layouting univarian…
the8472 Feb 16, 2023
4907dac
don't promote large fields to higher alignments if that would affect …
the8472 Mar 5, 2023
351e208
add tracing for layout optimizations
the8472 Mar 5, 2023
1a51ec6
bless tests
the8472 Feb 16, 2023
67a835d
fix codegen test
the8472 Apr 5, 2023
afe106c
[review] add comments, turn flag into enum
the8472 Apr 19, 2023
61fb5a9
layout-alignment-promotion logic should depend on the niche-bias
the8472 Apr 22, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
581 changes: 371 additions & 210 deletions compiler/rustc_abi/src/layout.rs
View file
Open in desktop

Large diffs are not rendered by default.

1 change: 1 addition & 0 deletions tests/codegen/issues/issue-103840.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
// compile-flags: -O
// min-llvm-version: 16.0
#![crate_type = "lib"]

pub fn foo(t: &mut Vec<usize>) {
Expand Down
3 changes: 2 additions & 1 deletion tests/codegen/issues/issue-105386-ub-in-debuginfo.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,4 +19,5 @@ pub fn outer_function(x: S, y: S) -> usize {
// CHECK-NOT: [[ptr_tmp:%.*]] = getelementptr inbounds %"[closure@{{.*.rs}}:9:23: 9:25]", ptr [[spill]]
// CHECK-NOT: [[load:%.*]] = load ptr, ptr
// CHECK: call void @llvm.lifetime.start{{.*}}({{.*}}, ptr [[spill]])
// CHECK: call void @llvm.memcpy{{.*}}(ptr {{align .*}} [[spill]], ptr {{align .*}} %x
// CHECK: [[inner:%.*]] = getelementptr inbounds %"{{.*}}", ptr [[spill]]
// CHECK: call void @llvm.memcpy{{.*}}(ptr {{align .*}} [[inner]], ptr {{align .*}} %x
29 changes: 9 additions & 20 deletions tests/codegen/issues/issue-86106.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
// min-llvm-version: 15.0
// only-64bit llvm appears to use stores instead of memset on 32bit
// compile-flags: -C opt-level=3 -Z merge-functions=disabled

// The below two functions ensure that both `String::new()` and `"".to_string()`
Expand All @@ -9,25 +10,19 @@
// CHECK-LABEL: define void @string_new
#[no_mangle]
pub fn string_new() -> String {
// CHECK-NOT: load i8
// CHECK: store i{{32|64}}
// CHECK: store ptr inttoptr
// CHECK-NEXT: getelementptr
// CHECK-NEXT: store ptr
// CHECK-NEXT: getelementptr
// CHECK-NEXT: store i{{32|64}}
// CHECK-NEXT: call void @llvm.memset
// CHECK-NEXT: ret void
String::new()
}

// CHECK-LABEL: define void @empty_to_string
#[no_mangle]
pub fn empty_to_string() -> String {
// CHECK-NOT: load i8
// CHECK: store i{{32|64}}
// CHECK-NEXT: getelementptr
// CHECK-NEXT: store ptr
// CHECK: store ptr inttoptr
// CHECK-NEXT: getelementptr
// CHECK-NEXT: store i{{32|64}}
// CHECK-NEXT: call void @llvm.memset
// CHECK-NEXT: ret void
"".to_string()
}
Expand All @@ -38,25 +33,19 @@ pub fn empty_to_string() -> String {
// CHECK-LABEL: @empty_vec
#[no_mangle]
pub fn empty_vec() -> Vec<u8> {
// CHECK: store i{{32|64}}
// CHECK-NOT: load i8
// CHECK: store ptr inttoptr
// CHECK-NEXT: getelementptr
// CHECK-NEXT: store ptr
// CHECK-NEXT: getelementptr
// CHECK-NEXT: store i{{32|64}}
// CHECK-NEXT: call void @llvm.memset
// CHECK-NEXT: ret void
vec![]
}

// CHECK-LABEL: @empty_vec_clone
#[no_mangle]
pub fn empty_vec_clone() -> Vec<u8> {
// CHECK: store i{{32|64}}
// CHECK-NOT: load i8
// CHECK-NEXT: getelementptr
// CHECK-NEXT: store ptr
// CHECK: store ptr inttoptr
// CHECK-NEXT: getelementptr
// CHECK-NEXT: store i{{32|64}}
// CHECK-NEXT: call void @llvm.memset
// CHECK-NEXT: ret void
vec![].clone()
}
40 changes: 18 additions & 22 deletions tests/ui/async-await/future-sizes/async-awaiting-fut.stdout
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,38 +2,34 @@ print-type-size type: `[async fn body@$DIR/async-awaiting-fut.rs:21:21: 24:2]`:
print-type-size discriminant: 1 bytes
print-type-size variant `Unresumed`: 0 bytes
print-type-size variant `Suspend0`: 3077 bytes
print-type-size local `.__awaitee`: 3077 bytes, offset: 0 bytes, alignment: 1 bytes
print-type-size local `.__awaitee`: 3077 bytes
print-type-size variant `Returned`: 0 bytes
print-type-size variant `Panicked`: 0 bytes
print-type-size type: `[async fn body@$DIR/async-awaiting-fut.rs:10:64: 19:2]`: 3077 bytes, alignment: 1 bytes
print-type-size discriminant: 1 bytes
print-type-size variant `Unresumed`: 2051 bytes
print-type-size padding: 1026 bytes
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👀

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hrm, not sure how to judge that one. It's a variant in an explicit-discriminant enum and I didn't pay much attention to those since they don't benefit from niche optimization. At least in this particular case it doesn't affect the overall enum size.

I also don't understand the meaning of , offset: 0 bytes, alignment: 1 bytes disappearing on other variants.

Should I investigate this?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a big improvement for the variant, so nothing to investigate. Although yeah, it doesn't matter at all (and could even just be print-type-size variant size calculation being odd).

print-type-size upvar `.fut`: 1025 bytes, alignment: 1 bytes
print-type-size variant `Unresumed`: 1025 bytes
print-type-size upvar `.fut`: 1025 bytes, offset: 0 bytes, alignment: 1 bytes
print-type-size variant `Suspend0`: 2052 bytes
print-type-size local `.fut`: 1025 bytes, offset: 0 bytes, alignment: 1 bytes
print-type-size local `..generator_field4`: 1 bytes
print-type-size upvar `.fut`: 1025 bytes, offset: 0 bytes, alignment: 1 bytes
print-type-size padding: 1 bytes
print-type-size upvar `.fut`: 1025 bytes, alignment: 1 bytes
print-type-size local `.fut`: 1025 bytes, alignment: 1 bytes
print-type-size local `..generator_field4`: 1 bytes
print-type-size local `.__awaitee`: 1 bytes
print-type-size variant `Suspend1`: 3076 bytes
print-type-size padding: 1024 bytes
print-type-size upvar `.fut`: 1025 bytes, offset: 0 bytes, alignment: 1 bytes
print-type-size padding: 1026 bytes
print-type-size local `..generator_field4`: 1 bytes, alignment: 1 bytes
print-type-size padding: 1 bytes
print-type-size upvar `.fut`: 1025 bytes, alignment: 1 bytes
print-type-size local `.__awaitee`: 1025 bytes
print-type-size variant `Suspend2`: 2052 bytes
print-type-size local `.fut`: 1025 bytes, offset: 0 bytes, alignment: 1 bytes
print-type-size local `..generator_field4`: 1 bytes
print-type-size upvar `.fut`: 1025 bytes, offset: 0 bytes, alignment: 1 bytes
print-type-size padding: 1 bytes
print-type-size upvar `.fut`: 1025 bytes, alignment: 1 bytes
print-type-size local `.fut`: 1025 bytes, alignment: 1 bytes
print-type-size local `..generator_field4`: 1 bytes
print-type-size local `.__awaitee`: 1 bytes
print-type-size variant `Returned`: 2051 bytes
print-type-size padding: 1026 bytes
print-type-size upvar `.fut`: 1025 bytes, alignment: 1 bytes
print-type-size variant `Panicked`: 2051 bytes
print-type-size padding: 1026 bytes
print-type-size upvar `.fut`: 1025 bytes, alignment: 1 bytes
print-type-size variant `Returned`: 1025 bytes
print-type-size upvar `.fut`: 1025 bytes, offset: 0 bytes, alignment: 1 bytes
print-type-size variant `Panicked`: 1025 bytes
print-type-size upvar `.fut`: 1025 bytes, offset: 0 bytes, alignment: 1 bytes
print-type-size type: `std::mem::ManuallyDrop<[async fn body@$DIR/async-awaiting-fut.rs:10:64: 19:2]>`: 3077 bytes, alignment: 1 bytes
print-type-size field `.value`: 3077 bytes
print-type-size type: `std::mem::MaybeUninit<[async fn body@$DIR/async-awaiting-fut.rs:10:64: 19:2]>`: 3077 bytes, alignment: 1 bytes
Expand All @@ -43,11 +39,11 @@ print-type-size field `.value`: 3077 bytes
print-type-size type: `[async fn body@$DIR/async-awaiting-fut.rs:8:35: 8:37]`: 1025 bytes, alignment: 1 bytes
print-type-size discriminant: 1 bytes
print-type-size variant `Unresumed`: 1024 bytes
print-type-size upvar `.arg`: 1024 bytes, offset: 0 bytes, alignment: 1 bytes
print-type-size upvar `.arg`: 1024 bytes
print-type-size variant `Returned`: 1024 bytes
print-type-size upvar `.arg`: 1024 bytes, offset: 0 bytes, alignment: 1 bytes
print-type-size upvar `.arg`: 1024 bytes
print-type-size variant `Panicked`: 1024 bytes
print-type-size upvar `.arg`: 1024 bytes, offset: 0 bytes, alignment: 1 bytes
print-type-size upvar `.arg`: 1024 bytes
print-type-size type: `std::mem::ManuallyDrop<[async fn body@$DIR/async-awaiting-fut.rs:8:35: 8:37]>`: 1025 bytes, alignment: 1 bytes
print-type-size field `.value`: 1025 bytes
print-type-size type: `std::mem::MaybeUninit<[async fn body@$DIR/async-awaiting-fut.rs:8:35: 8:37]>`: 1025 bytes, alignment: 1 bytes
Expand Down
24 changes: 12 additions & 12 deletions tests/ui/async-await/future-sizes/large-arg.stdout
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,20 +2,20 @@ print-type-size type: `[async fn body@$DIR/large-arg.rs:6:21: 8:2]`: 3076 bytes,
print-type-size discriminant: 1 bytes
print-type-size variant `Unresumed`: 0 bytes
print-type-size variant `Suspend0`: 3075 bytes
print-type-size local `.__awaitee`: 3075 bytes, offset: 0 bytes, alignment: 1 bytes
print-type-size local `.__awaitee`: 3075 bytes
print-type-size variant `Returned`: 0 bytes
print-type-size variant `Panicked`: 0 bytes
print-type-size type: `[async fn body@$DIR/large-arg.rs:10:30: 12:2]`: 3075 bytes, alignment: 1 bytes
print-type-size discriminant: 1 bytes
print-type-size variant `Unresumed`: 1024 bytes
print-type-size upvar `.t`: 1024 bytes, offset: 0 bytes, alignment: 1 bytes
print-type-size upvar `.t`: 1024 bytes
print-type-size variant `Suspend0`: 3074 bytes
print-type-size upvar `.t`: 1024 bytes, offset: 0 bytes, alignment: 1 bytes
print-type-size upvar `.t`: 1024 bytes
print-type-size local `.__awaitee`: 2050 bytes
print-type-size variant `Returned`: 1024 bytes
print-type-size upvar `.t`: 1024 bytes, offset: 0 bytes, alignment: 1 bytes
print-type-size upvar `.t`: 1024 bytes
print-type-size variant `Panicked`: 1024 bytes
print-type-size upvar `.t`: 1024 bytes, offset: 0 bytes, alignment: 1 bytes
print-type-size upvar `.t`: 1024 bytes
print-type-size type: `std::mem::ManuallyDrop<[async fn body@$DIR/large-arg.rs:10:30: 12:2]>`: 3075 bytes, alignment: 1 bytes
print-type-size field `.value`: 3075 bytes
print-type-size type: `std::mem::MaybeUninit<[async fn body@$DIR/large-arg.rs:10:30: 12:2]>`: 3075 bytes, alignment: 1 bytes
Expand All @@ -25,14 +25,14 @@ print-type-size field `.value`: 3075 bytes
print-type-size type: `[async fn body@$DIR/large-arg.rs:13:26: 15:2]`: 2050 bytes, alignment: 1 bytes
print-type-size discriminant: 1 bytes
print-type-size variant `Unresumed`: 1024 bytes
print-type-size upvar `.t`: 1024 bytes, offset: 0 bytes, alignment: 1 bytes
print-type-size upvar `.t`: 1024 bytes
print-type-size variant `Suspend0`: 2049 bytes
print-type-size upvar `.t`: 1024 bytes, offset: 0 bytes, alignment: 1 bytes
print-type-size upvar `.t`: 1024 bytes
print-type-size local `.__awaitee`: 1025 bytes
print-type-size variant `Returned`: 1024 bytes
print-type-size upvar `.t`: 1024 bytes, offset: 0 bytes, alignment: 1 bytes
print-type-size upvar `.t`: 1024 bytes
print-type-size variant `Panicked`: 1024 bytes
print-type-size upvar `.t`: 1024 bytes, offset: 0 bytes, alignment: 1 bytes
print-type-size upvar `.t`: 1024 bytes
print-type-size type: `std::mem::ManuallyDrop<[async fn body@$DIR/large-arg.rs:13:26: 15:2]>`: 2050 bytes, alignment: 1 bytes
print-type-size field `.value`: 2050 bytes
print-type-size type: `std::mem::MaybeUninit<[async fn body@$DIR/large-arg.rs:13:26: 15:2]>`: 2050 bytes, alignment: 1 bytes
Expand All @@ -42,11 +42,11 @@ print-type-size field `.value`: 2050 bytes
print-type-size type: `[async fn body@$DIR/large-arg.rs:16:26: 18:2]`: 1025 bytes, alignment: 1 bytes
print-type-size discriminant: 1 bytes
print-type-size variant `Unresumed`: 1024 bytes
print-type-size upvar `.t`: 1024 bytes, offset: 0 bytes, alignment: 1 bytes
print-type-size upvar `.t`: 1024 bytes
print-type-size variant `Returned`: 1024 bytes
print-type-size upvar `.t`: 1024 bytes, offset: 0 bytes, alignment: 1 bytes
print-type-size upvar `.t`: 1024 bytes
print-type-size variant `Panicked`: 1024 bytes
print-type-size upvar `.t`: 1024 bytes, offset: 0 bytes, alignment: 1 bytes
print-type-size upvar `.t`: 1024 bytes
print-type-size type: `std::mem::ManuallyDrop<[async fn body@$DIR/large-arg.rs:16:26: 18:2]>`: 1025 bytes, alignment: 1 bytes
print-type-size field `.value`: 1025 bytes
print-type-size type: `std::mem::MaybeUninit<[async fn body@$DIR/large-arg.rs:16:26: 18:2]>`: 1025 bytes, alignment: 1 bytes
Expand Down
4 changes: 2 additions & 2 deletions tests/ui/consts/const-eval/raw-bytes.32bit.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -465,7 +465,7 @@ LL | const LAYOUT_INVALID_ZERO: Layout = unsafe { Layout::from_size_align_unchec
|
= note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rustc repository if you believe it should not be considered undefined behavior.
= note: the raw bytes of the constant (size: 8, align: 4) {
00 10 00 00 00 00 00 00 │ ........
00 00 00 00 00 10 00 00 │ ........
}

error[E0080]: it is undefined behavior to use this value
Expand All @@ -476,7 +476,7 @@ LL | const LAYOUT_INVALID_THREE: Layout = unsafe { Layout::from_size_align_unche
|
= note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rustc repository if you believe it should not be considered undefined behavior.
= note: the raw bytes of the constant (size: 8, align: 4) {
09 00 00 00 03 00 00 00 │ ........
03 00 00 00 09 00 00 00 │ ........
}

error[E0080]: it is undefined behavior to use this value
Expand Down
4 changes: 2 additions & 2 deletions tests/ui/consts/const-eval/raw-bytes.64bit.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -465,7 +465,7 @@ LL | const LAYOUT_INVALID_ZERO: Layout = unsafe { Layout::from_size_align_unchec
|
= note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rustc repository if you believe it should not be considered undefined behavior.
= note: the raw bytes of the constant (size: 16, align: 8) {
00 10 00 00 00 00 00 00 00 00 00 00 00 00 00 00 │ ................
00 00 00 00 00 00 00 00 00 10 00 00 00 00 00 00 │ ................
}

error[E0080]: it is undefined behavior to use this value
Expand All @@ -476,7 +476,7 @@ LL | const LAYOUT_INVALID_THREE: Layout = unsafe { Layout::from_size_align_unche
|
= note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rustc repository if you believe it should not be considered undefined behavior.
= note: the raw bytes of the constant (size: 16, align: 8) {
09 00 00 00 00 00 00 00 03 00 00 00 00 00 00 00 │ ................
03 00 00 00 00 00 00 00 09 00 00 00 00 00 00 00 │ ................
}

error[E0080]: it is undefined behavior to use this value
Expand Down
40 changes: 20 additions & 20 deletions tests/ui/layout/issue-96158-scalarpair-payload-might-be-uninit.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -370,31 +370,31 @@ error: layout_of(NicheFirst) = Layout {
pref: $PREF_ALIGN,
},
abi: ScalarPair(
Union {
Initialized {
value: Int(
I8,
false,
),
valid_range: 0..=4,
},
Initialized {
Union {
value: Int(
I8,
false,
),
valid_range: 0..=4,
},
),
fields: Arbitrary {
offsets: [
Size(1 bytes),
Size(0 bytes),
],
memory_index: [
0,
],
},
largest_niche: Some(
Niche {
offset: Size(1 bytes),
offset: Size(0 bytes),
value: Int(
I8,
false,
Expand Down Expand Up @@ -429,29 +429,29 @@ error: layout_of(NicheFirst) = Layout {
I8,
false,
),
valid_range: 0..=255,
valid_range: 0..=2,
},
Initialized {
value: Int(
I8,
false,
),
valid_range: 0..=2,
valid_range: 0..=255,
},
),
fields: Arbitrary {
offsets: [
Size(1 bytes),
Size(0 bytes),
Size(1 bytes),
],
memory_index: [
1,
0,
1,
],
},
largest_niche: Some(
Niche {
offset: Size(1 bytes),
offset: Size(0 bytes),
value: Int(
I8,
false,
Expand Down Expand Up @@ -514,31 +514,31 @@ error: layout_of(NicheSecond) = Layout {
pref: $PREF_ALIGN,
},
abi: ScalarPair(
Union {
Initialized {
value: Int(
I8,
false,
),
valid_range: 0..=4,
},
Initialized {
Union {
value: Int(
I8,
false,
),
valid_range: 0..=4,
},
),
fields: Arbitrary {
offsets: [
Size(1 bytes),
Size(0 bytes),
],
memory_index: [
0,
],
},
largest_niche: Some(
Niche {
offset: Size(1 bytes),
offset: Size(0 bytes),
value: Int(
I8,
false,
Expand Down Expand Up @@ -573,29 +573,29 @@ error: layout_of(NicheSecond) = Layout {
I8,
false,
),
valid_range: 0..=255,
valid_range: 0..=2,
},
Initialized {
value: Int(
I8,
false,
),
valid_range: 0..=2,
valid_range: 0..=255,
},
),
fields: Arbitrary {
offsets: [
Size(0 bytes),
Size(1 bytes),
Size(0 bytes),
],
memory_index: [
0,
1,
0,
],
},
largest_niche: Some(
Niche {
offset: Size(1 bytes),
offset: Size(0 bytes),
value: Int(
I8,
false,
Expand Down
Loading