From b2c717fa338dd3e917008484c4bf55886041f743 Mon Sep 17 00:00:00 2001 From: Scott McMurray Date: Sat, 11 Mar 2023 15:32:54 -0800 Subject: `MaybeUninit::assume_init_read` should have `noundef` load metadata I was looking into `array::IntoIter` optimization, and noticed that it wasn't annotating the loads with `noundef` for simple things like `array::IntoIter`. Turned out to be a more general problem as `MaybeUninit::assume_init_read` isn't marking the load as initialized (), which is unfortunate since that's basically its reason to exist. This PR lowers `ptr::read(p)` to `copy *p` in MIR, which fortuitiously also improves the IR we give to LLVM for things like `mem::replace`. --- tests/codegen/mem-replace-direct-memcpy.rs | 41 +++++++++++++++++++++--- tests/codegen/read-noundef-metadata.rs | 51 ++++++++++++++++++++++++++++++ 2 files changed, 87 insertions(+), 5 deletions(-) create mode 100644 tests/codegen/read-noundef-metadata.rs (limited to 'tests/codegen') diff --git a/tests/codegen/mem-replace-direct-memcpy.rs b/tests/codegen/mem-replace-direct-memcpy.rs index e8bbf0e1bbd..3b01a621b56 100644 --- a/tests/codegen/mem-replace-direct-memcpy.rs +++ b/tests/codegen/mem-replace-direct-memcpy.rs @@ -12,13 +12,44 @@ pub fn replace_byte(dst: &mut u8, src: u8) -> u8 { std::mem::replace(dst, src) } +#[repr(C, align(8))] +pub struct Big([u64; 7]); +pub fn replace_big(dst: &mut Big, src: Big) -> Big { + // Before the `read_via_copy` intrinsic, this emitted six `memcpy`s. + std::mem::replace(dst, src) +} + // NOTE(eddyb) the `CHECK-NOT`s ensure that the only calls of `@llvm.memcpy` in -// the entire output, are the two direct calls we want, from `ptr::replace`. +// the entire output, are the direct calls we want, from `ptr::replace`. // CHECK-NOT: call void @llvm.memcpy -// CHECK: ; core::mem::replace -// CHECK-NOT: call void @llvm.memcpy -// CHECK: call void @llvm.memcpy.{{.+}}({{i8\*|ptr}} align 1 %{{.*}}, {{i8\*|ptr}} align 1 %{{.*}}, i{{.*}} 1, i1 false) + +// For a large type, we expect exactly three `memcpy`s +// CHECK-LABEL: define internal void @{{.+}}mem{{.+}}replace{{.+}}sret(%Big) + // CHECK-NOT: alloca + // CHECK: alloca %Big + // CHECK-NOT: alloca + // CHECK-NOT: call void @llvm.memcpy + // CHECK: call void @llvm.memcpy.{{.+}}({{i8\*|ptr}} align 8 %{{.*}}, {{i8\*|ptr}} align 8 %{{.*}}, i{{.*}} 56, i1 false) + // CHECK-NOT: call void @llvm.memcpy + // CHECK: call void @llvm.memcpy.{{.+}}({{i8\*|ptr}} align 8 %{{.*}}, {{i8\*|ptr}} align 8 %{{.*}}, i{{.*}} 56, i1 false) + // CHECK-NOT: call void @llvm.memcpy + // CHECK: call void @llvm.memcpy.{{.+}}({{i8\*|ptr}} align 8 %{{.*}}, {{i8\*|ptr}} align 8 %{{.*}}, i{{.*}} 56, i1 false) + // CHECK-NOT: call void @llvm.memcpy + // CHECK-NOT: call void @llvm.memcpy -// CHECK: call void @llvm.memcpy.{{.+}}({{i8\*|ptr}} align 1 %{{.*}}, {{i8\*|ptr}} align 1 %{{.*}}, i{{.*}} 1, i1 false) + +// For a small type, we expect one each of `load`/`store`/`memcpy` instead +// CHECK-LABEL: define internal noundef i8 @{{.+}}mem{{.+}}replace + // CHECK-NOT: alloca + // CHECK: alloca i8 + // CHECK-NOT: alloca + // CHECK-NOT: call void @llvm.memcpy + // CHECK: load i8 + // CHECK-NOT: call void @llvm.memcpy + // CHECK: store i8 + // CHECK-NOT: call void @llvm.memcpy + // CHECK: call void @llvm.memcpy.{{.+}}({{i8\*|ptr}} align 1 %{{.*}}, {{i8\*|ptr}} align 1 %{{.*}}, i{{.*}} 1, i1 false) + // CHECK-NOT: call void @llvm.memcpy + // CHECK-NOT: call void @llvm.memcpy diff --git a/tests/codegen/read-noundef-metadata.rs b/tests/codegen/read-noundef-metadata.rs new file mode 100644 index 00000000000..03386921c43 --- /dev/null +++ b/tests/codegen/read-noundef-metadata.rs @@ -0,0 +1,51 @@ +// compile-flags: -O -Z merge-functions=disabled +// no-system-llvm +// ignore-debug (the extra assertions get in the way) + +#![crate_type = "lib"] + +// Ensure that various forms of reading pointers correctly annotate the `load`s +// with `!noundef` metadata to enable extra optimization. The functions return +// `MaybeUninit` to keep it from being inferred from the function type. + +use std::mem::MaybeUninit; + +// CHECK-LABEL: define i8 @copy_byte( +#[no_mangle] +pub unsafe fn copy_byte(p: *const u8) -> MaybeUninit { + // CHECK-NOT: load + // CHECK: load i8, ptr %p, align 1 + // CHECK-SAME: !noundef ! + // CHECK-NOT: load + MaybeUninit::new(*p) +} + +// CHECK-LABEL: define i8 @read_byte( +#[no_mangle] +pub unsafe fn read_byte(p: *const u8) -> MaybeUninit { + // CHECK-NOT: load + // CHECK: load i8, ptr %p, align 1 + // CHECK-SAME: !noundef ! + // CHECK-NOT: load + MaybeUninit::new(p.read()) +} + +// CHECK-LABEL: define i8 @read_byte_maybe_uninit( +#[no_mangle] +pub unsafe fn read_byte_maybe_uninit(p: *const MaybeUninit) -> MaybeUninit { + // CHECK-NOT: load + // CHECK: load i8, ptr %p, align 1 + // CHECK-NOT: noundef + // CHECK-NOT: load + p.read() +} + +// CHECK-LABEL: define i8 @read_byte_assume_init( +#[no_mangle] +pub unsafe fn read_byte_assume_init(p: &MaybeUninit) -> MaybeUninit { + // CHECK-NOT: load + // CHECK: load i8, ptr %p, align 1 + // CHECK-SAME: !noundef ! + // CHECK-NOT: load + MaybeUninit::new(p.assume_init_read()) +} -- cgit 1.4.1-3-g733a5