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

Add !align metadata on loads of &/&mut/Box #94159

Merged
merged 3 commits into from
Mar 4, 2022
Merged

Add !align metadata on loads of &/&mut/Box #94159

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
69ae423
Add !align metadata on loads of &/&mut/Box
erikdesjardins Feb 16, 2022
f147303
fix tests on platforms where Align16 is represented as i128
erikdesjardins Mar 2, 2022
a381aef
make test work on noopt builder
erikdesjardins Mar 3, 2022
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
38 changes: 30 additions & 8 deletions compiler/rustc_codegen_llvm/src/builder.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -478,6 +478,8 @@ impl<'a, 'll, 'tcx> BuilderMethods<'a, 'tcx> for Builder<'a, 'll, 'tcx> {
bx: &mut Builder<'a, 'll, 'tcx>,
load: &'ll Value,
scalar: abi::Scalar,
layout: TyAndLayout<'tcx>,
offset: Size,
) {
if !scalar.is_always_valid(bx) {
bx.noundef_metadata(load);
Expand All @@ -489,10 +491,18 @@ impl<'a, 'll, 'tcx> BuilderMethods<'a, 'tcx> for Builder<'a, 'll, 'tcx> {
bx.range_metadata(load, scalar.valid_range);
}
}
abi::Pointer if !scalar.valid_range.contains(0) => {
bx.nonnull_metadata(load);
abi::Pointer => {
if !scalar.valid_range.contains(0) {
bx.nonnull_metadata(load);
}

if let Some(pointee) = layout.pointee_info_at(bx, offset) {
if let Some(_) = pointee.safe {
bx.align_metadata(load, pointee.align);
}
}
}
_ => {}
abi::F32 | abi::F64 => {}
}
}

Expand All @@ -510,7 +520,7 @@ impl<'a, 'll, 'tcx> BuilderMethods<'a, 'tcx> for Builder<'a, 'll, 'tcx> {
let llval = const_llval.unwrap_or_else(|| {
let load = self.load(place.layout.llvm_type(self), place.llval, place.align);
if let abi::Abi::Scalar(scalar) = place.layout.abi {
scalar_load_metadata(self, load, scalar);
scalar_load_metadata(self, load, scalar, place.layout, Size::ZERO);
}
load
});
Expand All @@ -519,17 +529,17 @@ impl<'a, 'll, 'tcx> BuilderMethods<'a, 'tcx> for Builder<'a, 'll, 'tcx> {
let b_offset = a.value.size(self).align_to(b.value.align(self).abi);
let pair_ty = place.layout.llvm_type(self);

let mut load = |i, scalar: abi::Scalar, align| {
let mut load = |i, scalar: abi::Scalar, layout, align, offset| {
let llptr = self.struct_gep(pair_ty, place.llval, i as u64);
let llty = place.layout.scalar_pair_element_llvm_type(self, i, false);
let load = self.load(llty, llptr, align);
scalar_load_metadata(self, load, scalar);
scalar_load_metadata(self, load, scalar, layout, offset);
self.to_immediate_scalar(load, scalar)
};

OperandValue::Pair(
load(0, a, place.align),
load(1, b, place.align.restrict_for_offset(b_offset)),
load(0, a, place.layout, place.align, Size::ZERO),
load(1, b, place.layout, place.align.restrict_for_offset(b_offset), b_offset),
nikic marked this conversation as resolved.
Show resolved Hide resolved
)
} else {
OperandValue::Ref(place.llval, None, place.align)
Expand Down Expand Up @@ -1219,6 +1229,18 @@ impl<'a, 'll, 'tcx> Builder<'a, 'll, 'tcx> {
}
}

fn align_metadata(&mut self, load: &'ll Value, align: Align) {
unsafe {
let v = [self.cx.const_u64(align.bytes())];

llvm::LLVMSetMetadata(
load,
llvm::MD_align as c_uint,
llvm::LLVMMDNodeInContext(self.cx.llcx, v.as_ptr(), v.len() as c_uint),
);
}
}

fn noundef_metadata(&mut self, load: &'ll Value) {
unsafe {
llvm::LLVMSetMetadata(
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_codegen_llvm/src/llvm/ffi.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -441,6 +441,7 @@ pub enum MetadataType {
MD_nontemporal = 9,
MD_mem_parallel_loop_access = 10,
MD_nonnull = 11,
MD_align = 17,
MD_type = 19,
MD_noundef = 29,
}
Expand Down
44 changes: 36 additions & 8 deletions src/test/codegen/loads.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,21 +18,47 @@ pub enum MyBool {
False,
}

#[repr(align(16))]
pub struct Align16(u128);

// CHECK: @ptr_alignment_helper({}** {{.*}}align [[PTR_ALIGNMENT:[0-9]+]]
#[no_mangle]
pub fn ptr_alignment_helper(x: &&()) {}

// CHECK-LABEL: @load_ref
#[no_mangle]
pub fn load_ref<'a>(x: &&'a i32) -> &'a i32 {
// Alignment of a reference itself is target dependent, so just match any alignment:
// the main thing we care about here is !nonnull and !noundef.
// CHECK: load i32*, i32** %x, align {{[0-9]+}}, !nonnull !{{[0-9]+}}, !noundef !{{[0-9]+}}
// CHECK: load i32*, i32** %x, align [[PTR_ALIGNMENT]], !nonnull !{{[0-9]+}}, !align ![[ALIGN_4_META:[0-9]+]], !noundef !{{[0-9]+}}
*x
}

// CHECK-LABEL: @load_ref_higher_alignment
#[no_mangle]
pub fn load_ref_higher_alignment<'a>(x: &&'a Align16) -> &'a Align16 {
// CHECK: load {{%Align16|i128}}*, {{%Align16|i128}}** %x, align [[PTR_ALIGNMENT]], !nonnull !{{[0-9]+}}, !align ![[ALIGN_16_META:[0-9]+]], !noundef !{{[0-9]+}}
*x
}

// CHECK-LABEL: @load_scalar_pair
#[no_mangle]
pub fn load_scalar_pair<'a>(x: &(&'a i32, &'a Align16)) -> (&'a i32, &'a Align16) {
// CHECK: load i32*, i32** %{{.+}}, align [[PTR_ALIGNMENT]], !nonnull !{{[0-9]+}}, !align ![[ALIGN_4_META]], !noundef !{{[0-9]+}}
// CHECK: load i64*, i64** %{{.+}}, align [[PTR_ALIGNMENT]], !nonnull !{{[0-9]+}}, !align ![[ALIGN_16_META]], !noundef !{{[0-9]+}}
*x
}

// CHECK-LABEL: @load_raw_pointer
#[no_mangle]
pub fn load_raw_pointer<'a>(x: &*const i32) -> *const i32 {
// loaded raw pointer should not have !nonnull, !align, or !noundef metadata
// CHECK: load i32*, i32** %x, align [[PTR_ALIGNMENT]]{{$}}
*x
}

// CHECK-LABEL: @load_box
#[no_mangle]
pub fn load_box<'a>(x: Box<Box<i32>>) -> Box<i32> {
// Alignment of a box itself is target dependent, so just match any alignment:
// the main thing we care about here is !nonnull and !noundef.
// CHECK: load i32*, i32** %x, align {{[0-9]+}}, !nonnull !{{[0-9]+}}, !noundef !{{[0-9]+}}
// CHECK: load i32*, i32** %x, align [[PTR_ALIGNMENT]], !nonnull !{{[0-9]+}}, !align ![[ALIGN_4_META]], !noundef !{{[0-9]+}}
*x
}

Expand Down Expand Up @@ -120,5 +146,7 @@ pub fn small_struct_alignment(x: Bytes) -> Bytes {
x
}

// CHECK: ![[BOOL_RANGE]] = !{i8 0, i8 2}
// CHECK: ![[NONZEROU16_RANGE]] = !{i16 1, i16 0}
// CHECK-DAG: ![[BOOL_RANGE]] = !{i8 0, i8 2}
// CHECK-DAG: ![[NONZEROU16_RANGE]] = !{i16 1, i16 0}
// CHECK-DAG: ![[ALIGN_4_META]] = !{i64 4}
// CHECK-DAG: ![[ALIGN_16_META]] = !{i64 16}