mikros/rust
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Improved implementation and comments after code review feedback

Browse Source
This commit is contained in:
Scott McMurray 2023-03-14 22:24:28 -07:00
parent 87696fd5a1
commit e7c6ad89cf
6 changed files with 124 additions and 77 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

31
compiler/rustc_mir_transform/src/lower_intrinsics.rs
View File

@ -150,7 +150,7 @@ fn run_pass(&self, tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) {
}
}
sym::read_via_copy => {
let Ok([arg]) = <[_; 1]>::try_from(std::mem::take(args)) else {
let [arg] = args.as_slice() else {
span_bug!(terminator.source_info.span, "Wrong number of arguments");
};
let derefed_place =
@ -159,18 +159,23 @@ fn run_pass(&self, tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) {
} else {
span_bug!(terminator.source_info.span, "Only passing a local is supported");
};
block.statements.push(Statement {
source_info: terminator.source_info,
kind: StatementKind::Assign(Box::new((
*destination,
Rvalue::Use(Operand::Copy(derefed_place)),
))),
});
if let Some(target) = *target {
terminator.kind = TerminatorKind::Goto { target };
} else {
// Reading something uninhabited means this is unreachable.
terminator.kind = TerminatorKind::Unreachable;
terminator.kind = match *target {
None => {
// No target means this read something uninhabited,
// so it must be unreachable, and we don't need to
// preserve the assignment either.
TerminatorKind::Unreachable
}
Some(target) => {
block.statements.push(Statement {
source_info: terminator.source_info,
kind: StatementKind::Assign(Box::new((
*destination,
Rvalue::Use(Operand::Copy(derefed_place)),
))),
});
TerminatorKind::Goto { target }
}
}
}
sym::discriminant_value => {

14
library/core/src/intrinsics.rs
View File

@ -2020,16 +2020,12 @@ pub unsafe fn drop_in_place<T: ?Sized>(to_drop: *mut T) {
#[rustc_safe_intrinsic]
pub fn saturating_sub<T: Copy>(a: T, b: T) -> T;
/// This is a *typed* read, `copy *p` in MIR.
/// This is an implementation detail of [`crate::ptr::read`] and should
/// not be used anywhere else. See its comments for why this exists.
///
/// The stabilized form of this intrinsic is [`crate::ptr::read`], so
/// that can be implemented without needing to do an *untyped* copy
/// via [`copy_nonoverlapping`], and thus can get proper metadata.
///
/// This intrinsic can *only* be called with a copy or move of a local.
/// (It allows neither constants nor projections.)
///
/// To avoid introducing any `noalias` requirements, it just takes a pointer.
/// This intrinsic can *only* be called where the argument is a local without
/// projections (`read_via_copy(p)`, not `read_via_copy(*p)`) so that it
/// trivially obeys runtime-MIR rules about derefs in operands.
#[cfg(not(bootstrap))]
#[rustc_const_unstable(feature = "const_ptr_read", issue = "80377")]
pub fn read_via_copy<T>(p: *const T) -> T;

8
library/core/src/ptr/mod.rs
View File

@ -1136,10 +1136,12 @@ macro_rules! attempt_swap_as_chunks {
#[cfg_attr(miri, track_caller)] // even without panics, this helps for Miri backtraces
pub const unsafe fn read<T>(src: *const T) -> T {
// It would be semantically correct to implement this via `copy_nonoverlapping`
// and `MaybeUninit`, as was done before PR #109035.
// and `MaybeUninit`, as was done before PR #109035. Calling `assume_init`
// provides enough information to know that this is a typed operation.
// However, it switched to intrinsic that lowers to `_0 = *src` in MIR in
// order to address a few implementation issues:
// However, as of March 2023 the compiler was not capable of taking advantage
// of that information. Thus the implementation here switched to an intrinsic,
// which lowers to `_0 = *src` in MIR, to address a few issues:
//
// - Using `MaybeUninit::assume_init` after a `copy_nonoverlapping` was not
// turning the untyped copy into a typed load. As such, the generated

96
tests/codegen/ptr-read-metadata.rs Normal file
View File

@ -0,0 +1,96 @@
// 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` and `!range` metadata to enable extra optimization.
use std::mem::MaybeUninit;
// CHECK-LABEL: define noundef i8 @copy_byte(
#[no_mangle]
pub unsafe fn copy_byte(p: *const u8) -> u8 {
// CHECK-NOT: load
// CHECK: load i8, ptr %p, align 1
// CHECK-SAME: !noundef !
// CHECK-NOT: load
*p
}
// CHECK-LABEL: define noundef i8 @read_byte(
#[no_mangle]
pub unsafe fn read_byte(p: *const u8) -> u8 {
// CHECK-NOT: load
// CHECK: load i8, ptr %p, align 1
// CHECK-SAME: !noundef !
// CHECK-NOT: load
p.read()
}
// CHECK-LABEL: define i8 @read_byte_maybe_uninit(
#[no_mangle]
pub unsafe fn read_byte_maybe_uninit(p: *const MaybeUninit<u8>) -> MaybeUninit<u8> {
// CHECK-NOT: load
// CHECK: load i8, ptr %p, align 1
// CHECK-NOT: noundef
// CHECK-NOT: load
p.read()
}
// CHECK-LABEL: define noundef i8 @read_byte_assume_init(
#[no_mangle]
pub unsafe fn read_byte_assume_init(p: &MaybeUninit<u8>) -> u8 {
// CHECK-NOT: load
// CHECK: load i8, ptr %p, align 1
// CHECK-SAME: !noundef !
// CHECK-NOT: load
p.assume_init_read()
}
// CHECK-LABEL: define noundef i32 @copy_char(
#[no_mangle]
pub unsafe fn copy_char(p: *const char) -> char {
// CHECK-NOT: load
// CHECK: load i32, ptr %p
// CHECK-SAME: !range ![[RANGE:[0-9]+]]
// CHECK-SAME: !noundef !
// CHECK-NOT: load
*p
}
// CHECK-LABEL: define noundef i32 @read_char(
#[no_mangle]
pub unsafe fn read_char(p: *const char) -> char {
// CHECK-NOT: load
// CHECK: load i32, ptr %p
// CHECK-SAME: !range ![[RANGE]]
// CHECK-SAME: !noundef !
// CHECK-NOT: load
p.read()
}
// CHECK-LABEL: define i32 @read_char_maybe_uninit(
#[no_mangle]
pub unsafe fn read_char_maybe_uninit(p: *const MaybeUninit<char>) -> MaybeUninit<char> {
// CHECK-NOT: load
// CHECK: load i32, ptr %p
// CHECK-NOT: range
// CHECK-NOT: noundef
// CHECK-NOT: load
p.read()
}
// CHECK-LABEL: define noundef i32 @read_char_assume_init(
#[no_mangle]
pub unsafe fn read_char_assume_init(p: &MaybeUninit<char>) -> char {
// CHECK-NOT: load
// CHECK: load i32, ptr %p
// CHECK-SAME: !range ![[RANGE]]
// CHECK-SAME: !noundef !
// CHECK-NOT: load
p.assume_init_read()
}
// CHECK: ![[RANGE]] = !{i32 0, i32 1114112}

51
tests/codegen/read-noundef-metadata.rs
View File

@ -1,51 +0,0 @@
// 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<u8> {
// 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<u8> {
// 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<u8>) -> MaybeUninit<u8> {
// 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<u8>) -> MaybeUninit<u8> {
// CHECK-NOT: load
// CHECK: load i8, ptr %p, align 1
// CHECK-SAME: !noundef !
// CHECK-NOT: load
MaybeUninit::new(p.assume_init_read())
}

1
tests/mir-opt/lower_intrinsics.read_via_copy_uninhabited.LowerIntrinsics.diff
View File

@ -15,7 +15,6 @@
- // mir::Constant
- // + span: $DIR/lower_intrinsics.rs:90:14: 90:45
- // + literal: Const { ty: unsafe extern "rust-intrinsic" fn(*const Never) -> Never {read_via_copy::<Never>}, val: Value(<ZST>) }
+ _0 = (*_2); // scope 1 at $DIR/lower_intrinsics.rs:+1:14: +1:48
+ unreachable; // scope 1 at $DIR/lower_intrinsics.rs:+1:14: +1:48
}
}