Skip to content

Commit

Permalink
Auto merge of rust-lang#128371 - andjo403:rangeAttribute, r=<try>
Browse files Browse the repository at this point in the history
Add range attribute to scalar function results and arguments

as LLVM 19 adds the range attribute this starts to use it for better optimization.
hade been interesting to see a perf run with the rust-lang#127513

closes rust-lang#50156
cc rust-lang#49572 shall be fixed but not possible to see as there is asserts that already trigger the optimization.

try-job: i686-gnu
  • Loading branch information
bors committed Aug 9, 2024
2 parents 97e7252 + 01a3604 commit c759157
Show file tree
Hide file tree
Showing 12 changed files with 159 additions and 24 deletions.
61 changes: 48 additions & 13 deletions compiler/rustc_codegen_llvm/src/abi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -430,9 +430,32 @@ impl<'ll, 'tcx> FnAbiLlvmExt<'ll, 'tcx> for FnAbi<'tcx, Ty<'tcx>> {
i += 1;
i - 1
};

let apply_range_attr = |idx: AttributePlace, scalar: rustc_target::abi::Scalar| {
if cx.sess().opts.optimize != config::OptLevel::No
&& llvm_util::get_version() >= (19, 0, 0)
&& matches!(scalar.primitive(), Int(..))
// If the value is a boolean, the range is 0..2 and that ultimately
// become 0..0 when the type becomes i1, which would be rejected
// by the LLVM verifier.
&& !scalar.is_bool()
// LLVM also rejects full range.
&& !scalar.is_always_valid(cx)
{
attributes::apply_to_llfn(
llfn,
idx,
&[llvm::CreateRangeAttr(cx.llcx, scalar.size(cx), scalar.valid_range(cx))],
);
}
};

match &self.ret.mode {
PassMode::Direct(attrs) => {
attrs.apply_attrs_to_llfn(llvm::AttributePlace::ReturnValue, cx, llfn);
if let abi::Abi::Scalar(scalar) = self.ret.layout.abi {
apply_range_attr(llvm::AttributePlace::ReturnValue, scalar);
}
}
PassMode::Indirect { attrs, meta_attrs: _, on_stack } => {
assert!(!on_stack);
Expand Down Expand Up @@ -471,8 +494,13 @@ impl<'ll, 'tcx> FnAbiLlvmExt<'ll, 'tcx> for FnAbi<'tcx, Ty<'tcx>> {
);
attributes::apply_to_llfn(llfn, llvm::AttributePlace::Argument(i), &[byval]);
}
PassMode::Direct(attrs)
| PassMode::Indirect { attrs, meta_attrs: None, on_stack: false } => {
PassMode::Direct(attrs) => {
let i = apply(attrs);
if let abi::Abi::Scalar(scalar) = arg.layout.abi {
apply_range_attr(llvm::AttributePlace::Argument(i), scalar);
}
}
PassMode::Indirect { attrs, meta_attrs: None, on_stack: false } => {
apply(attrs);
}
PassMode::Indirect { attrs, meta_attrs: Some(meta_attrs), on_stack } => {
Expand All @@ -481,8 +509,12 @@ impl<'ll, 'tcx> FnAbiLlvmExt<'ll, 'tcx> for FnAbi<'tcx, Ty<'tcx>> {
apply(meta_attrs);
}
PassMode::Pair(a, b) => {
apply(a);
apply(b);
let i = apply(a);
let ii = apply(b);
if let abi::Abi::ScalarPair(scalar_a, scalar_b) = arg.layout.abi {
apply_range_attr(llvm::AttributePlace::Argument(i), scalar_a);
apply_range_attr(llvm::AttributePlace::Argument(ii), scalar_b);
}
}
PassMode::Cast { cast, pad_i32 } => {
if *pad_i32 {
Expand Down Expand Up @@ -537,15 +569,18 @@ impl<'ll, 'tcx> FnAbiLlvmExt<'ll, 'tcx> for FnAbi<'tcx, Ty<'tcx>> {
}
_ => {}
}
if let abi::Abi::Scalar(scalar) = self.ret.layout.abi {
// If the value is a boolean, the range is 0..2 and that ultimately
// become 0..0 when the type becomes i1, which would be rejected
// by the LLVM verifier.
if let Int(..) = scalar.primitive() {
if !scalar.is_bool() && !scalar.is_always_valid(bx) {
bx.range_metadata(callsite, scalar.valid_range(bx));
}
}
if bx.cx.sess().opts.optimize != config::OptLevel::No
&& llvm_util::get_version() < (19, 0, 0)
&& let abi::Abi::Scalar(scalar) = self.ret.layout.abi
&& matches!(scalar.primitive(), Int(..))
// If the value is a boolean, the range is 0..2 and that ultimately
// become 0..0 when the type becomes i1, which would be rejected
// by the LLVM verifier.
&& !scalar.is_bool()
// LLVM also rejects full range.
&& !scalar.is_always_valid(bx)
{
bx.range_metadata(callsite, scalar.valid_range(bx));
}
for arg in self.args.iter() {
match &arg.mode {
Expand Down
6 changes: 6 additions & 0 deletions compiler/rustc_codegen_llvm/src/llvm/ffi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1575,6 +1575,12 @@ extern "C" {
pub fn LLVMRustCreateAllocSizeAttr(C: &Context, size_arg: u32) -> &Attribute;
pub fn LLVMRustCreateAllocKindAttr(C: &Context, size_arg: u64) -> &Attribute;
pub fn LLVMRustCreateMemoryEffectsAttr(C: &Context, effects: MemoryEffects) -> &Attribute;
pub fn LLVMRustCreateRangeAttribute(
C: &Context,
num_bits: c_uint,
lower_words: *const u64,
upper_words: *const u64,
) -> &Attribute;

// Operations on functions
pub fn LLVMRustGetOrInsertFunction<'a>(
Expand Down
17 changes: 16 additions & 1 deletion compiler/rustc_codegen_llvm/src/llvm/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use std::string::FromUtf8Error;
use libc::c_uint;
use rustc_data_structures::small_c_str::SmallCStr;
use rustc_llvm::RustString;
use rustc_target::abi::Align;
use rustc_target::abi::{Align, Size, WrappingRange};

pub use self::AtomicRmwBinOp::*;
pub use self::CallConv::*;
Expand Down Expand Up @@ -105,6 +105,21 @@ pub fn CreateAllocKindAttr(llcx: &Context, kind_arg: AllocKindFlags) -> &Attribu
unsafe { LLVMRustCreateAllocKindAttr(llcx, kind_arg.bits()) }
}

pub fn CreateRangeAttr(llcx: &Context, size: Size, range: WrappingRange) -> &Attribute {
let lower = range.start;
let upper = range.end.wrapping_add(1);
let lower_words = [lower as u64, (lower >> 64) as u64];
let upper_words = [upper as u64, (upper >> 64) as u64];
unsafe {
LLVMRustCreateRangeAttribute(
llcx,
size.bits().try_into().unwrap(),
lower_words.as_ptr(),
upper_words.as_ptr(),
)
}
}

#[derive(Copy, Clone)]
pub enum AttributePlace {
ReturnValue,
Expand Down
12 changes: 12 additions & 0 deletions compiler/rustc_llvm/llvm-wrapper/RustWrapper.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -397,6 +397,18 @@ LLVMRustCreateAllocSizeAttr(LLVMContextRef C, uint32_t ElementSizeArg) {
std::nullopt));
}

extern "C" LLVMAttributeRef
LLVMRustCreateRangeAttribute(LLVMContextRef C, unsigned NumBits,
const uint64_t LowerWords[],
const uint64_t UpperWords[]) {
#if LLVM_VERSION_GE(19, 0)
return LLVMCreateConstantRangeAttribute(C, Attribute::Range, NumBits,
LowerWords, UpperWords);
#else
report_fatal_error("LLVM 19.0 is required for Range Attribute");
#endif
}

// These values **must** match ffi::AllocKindFlags.
// It _happens_ to match the LLVM values of llvm::AllocFnKind,
// but that's happenstance and we do explicit conversions before
Expand Down
1 change: 1 addition & 0 deletions tests/codegen/call-metadata.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// scalar value.

//@ compile-flags: -O -C no-prepopulate-passes
//@ ignore-llvm-version: 19 - 99

#![crate_type = "lib"]

Expand Down
2 changes: 0 additions & 2 deletions tests/codegen/cast-optimized.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,6 @@ pub fn u32_index(c: u32) -> [bool; 22] {
// CHECK-LABEL: @char_as_u32_index
#[no_mangle]
pub fn char_as_u32_index(c: char) -> [bool; 22] {
// CHECK: %[[B:.+]] = icmp ult i32 %c, 1114112
// CHECK: call void @llvm.assume(i1 %[[B]])
let c = c as u32;

let mut array = [false; 22];
Expand Down
4 changes: 2 additions & 2 deletions tests/codegen/common_prim_int_ptr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ pub fn insert_box(x: Box<()>) -> Result<usize, Box<()>> {

// CHECK-LABEL: @extract_int
// CHECK-NOT: nonnull
// CHECK-SAME: (i{{[0-9]+}} {{[^,]+}} [[DISCRIMINANT:%[0-9]+]], ptr {{[^,]+}} [[PAYLOAD:%[0-9]+]])
// CHECK-SAME: (i{{[0-9]+}} {{[^%]+}} [[DISCRIMINANT:%[0-9]+]], ptr {{[^,]+}} [[PAYLOAD:%[0-9]+]])
#[no_mangle]
pub unsafe fn extract_int(x: Result<usize, Box<()>>) -> usize {
// CHECK: [[TEMP:%.+]] = ptrtoint ptr [[PAYLOAD]] to [[USIZE:i[0-9]+]]
Expand All @@ -40,7 +40,7 @@ pub unsafe fn extract_int(x: Result<usize, Box<()>>) -> usize {
}

// CHECK-LABEL: @extract_box
// CHECK-SAME: (i{{[0-9]+}} {{[^,]+}} [[DISCRIMINANT:%[0-9]+]], ptr {{[^,]+}} [[PAYLOAD:%[0-9]+]])
// CHECK-SAME: (i{{[0-9]+}} {{[^%]+}} [[DISCRIMINANT:%[0-9]+]], ptr {{[^,]+}} [[PAYLOAD:%[0-9]+]])
#[no_mangle]
pub unsafe fn extract_box(x: Result<usize, Box<i32>>) -> Box<i32> {
// CHECK: ret ptr [[PAYLOAD]]
Expand Down
2 changes: 1 addition & 1 deletion tests/codegen/enum/enum-match.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ pub enum Enum1 {

// CHECK: define noundef{{( range\(i8 [0-9]+, [0-9]+\))?}} i8 @match1{{.*}}
// CHECK-NEXT: start:
// CHECK-NEXT: %1 = add i8 %0, -2
// CHECK-NEXT: %1 = add{{( nsw)?}} i8 %0, -2
// CHECK-NEXT: %2 = zext i8 %1 to i64
// CHECK-NEXT: %3 = icmp ult i8 %1, 2
// CHECK-NEXT: %4 = add nuw nsw i64 %2, 1
Expand Down
6 changes: 3 additions & 3 deletions tests/codegen/function-arguments.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ pub fn maybeuninit_enum_bool(x: MaybeUninit<MyBool>) -> MaybeUninit<MyBool> {
x
}

// CHECK: noundef i32 @char(i32 noundef %x)
// CHECK: noundef{{( range\(i32 0, 1114112\))?}} i32 @char(i32 noundef{{( range\(i32 0, 1114112\))?}} %x)
#[no_mangle]
pub fn char(x: char) -> char {
x
Expand All @@ -68,7 +68,7 @@ pub fn int(x: u64) -> u64 {
x
}

// CHECK: noundef i64 @nonzero_int(i64 noundef %x)
// CHECK: noundef{{( range\(i64 1, 0\))?}} i64 @nonzero_int(i64 noundef{{( range\(i64 1, 0\))?}} %x)
#[no_mangle]
pub fn nonzero_int(x: NonZero<u64>) -> NonZero<u64> {
x
Expand Down Expand Up @@ -250,7 +250,7 @@ pub fn return_slice(x: &[u16]) -> &[u16] {
x
}

// CHECK: { i16, i16 } @enum_id_1(i16 noundef %x.0, i16 %x.1)
// CHECK: { i16, i16 } @enum_id_1(i16 noundef{{( range\(i16 0, 3\))?}} %x.0, i16 %x.1)
#[no_mangle]
pub fn enum_id_1(x: Option<Result<u16, u16>>) -> Option<Result<u16, u16>> {
x
Expand Down
2 changes: 1 addition & 1 deletion tests/codegen/issues/issue-68667-unwrap-combinators.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
// MIR inlining now optimizes this code.

// CHECK-LABEL: @unwrap_combinators
// CHECK: icmp
// CHECK: {{icmp|trunc}}
// CHECK-NEXT: icmp
// CHECK-NEXT: select i1
// CHECK-NEXT: ret i1
Expand Down
68 changes: 68 additions & 0 deletions tests/codegen/range-attribute.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
// Checks that range metadata gets emitted on functions result and arguments
// with scalar value.

//@ compile-flags: -O -C no-prepopulate-passes
//@ min-llvm-version: 19

#![crate_type = "lib"]

use std::num::NonZero;

// Hack to get the correct size for usize
// CHECK: @helper([[USIZE:i[0-9]+]] noundef %_1)
#[no_mangle]
pub fn helper(_: usize) {}

// CHECK: noundef range(i128 1, 0) i128 @nonzero_int(i128 noundef range(i128 1, 0) %x)
#[no_mangle]
pub fn nonzero_int(x: NonZero<u128>) -> NonZero<u128> {
x
}

// CHECK: noundef range(i8 0, 3) i8 @optional_bool(i8 noundef range(i8 0, 3) %x)
#[no_mangle]
pub fn optional_bool(x: Option<bool>) -> Option<bool> {
x
}

pub enum Enum0 {
A(bool),
B,
C,
}

// CHECK: noundef range(i8 0, 4) i8 @enum0_value(i8 noundef range(i8 0, 4) %x)
#[no_mangle]
pub fn enum0_value(x: Enum0) -> Enum0 {
x
}

pub enum Enum1 {
A(u64),
B(u64),
C(u64),
}

// CHECK: { [[USIZE]], i64 } @enum1_value([[USIZE]] noundef range([[USIZE]] 0, 3) %x.0, i64 noundef %x.1)
#[no_mangle]
pub fn enum1_value(x: Enum1) -> Enum1 {
x
}

pub enum Enum2 {
A(Enum0),
B(Enum0),
C(Enum0),
}

// CHECK: { i8, i8 } @enum2_value(i8 noundef range(i8 0, 3) %x.0, i8 noundef %x.1)
#[no_mangle]
pub fn enum2_value(x: Enum2) -> Enum2 {
x
}

// CHECK: noundef [[USIZE]] @takes_slice(ptr noalias noundef nonnull readonly align 4 %x.0, [[USIZE]] noundef %x.1)
#[no_mangle]
pub fn takes_slice(x: &[i32]) -> usize {
x.len()
}
2 changes: 1 addition & 1 deletion tests/codegen/repr/transparent.rs
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ pub enum Bool {
FileNotFound,
}

// CHECK: define{{( dso_local)?}} noundef{{( zeroext)?}} i8 @test_Gpz(i8 noundef{{( zeroext)?}} %_1)
// CHECK: define{{( dso_local)?}} noundef{{( zeroext)?( range\(i8 0, 3\))?}} i8 @test_Gpz(i8 noundef{{( zeroext)?( range\(i8 0, 3\))?}} %_1)
#[no_mangle]
pub extern "C" fn test_Gpz(_: GenericPlusZst<Bool>) -> GenericPlusZst<Bool> {
loop {}
Expand Down

0 comments on commit c759157

Please sign in to comment.