Skip to content

Commit

Permalink
Auto merge of #50164 - nox:rval-range-metadata, r=<try>
Browse files Browse the repository at this point in the history
Emit range metadata on calls returning scalars (fixes #50157)
  • Loading branch information
bors committed Apr 24, 2018
2 parents 2a6200a + c7c292b commit c4ba253
Show file tree
Hide file tree
Showing 6 changed files with 76 additions and 20 deletions.
21 changes: 20 additions & 1 deletion src/librustc/ty/layout.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ use std::fmt;
use std::i128;
use std::iter;
use std::mem;
use std::ops::{Add, Sub, Mul, AddAssign, Deref, RangeInclusive};
use std::ops::{Add, Sub, Mul, AddAssign, Deref, Range, RangeInclusive};

use ich::StableHashingContext;
use rustc_data_structures::stable_hasher::{HashStable, StableHasher,
Expand Down Expand Up @@ -648,6 +648,25 @@ impl Scalar {
false
}
}

/// Returns a range suitable to be passed to LLVM for range metadata.
pub fn range_metadata<C: HasDataLayout>(&self, cx: C) -> Option<Range<u128>> {
// For a (max) value of -1, max will be `-1 as usize`, which overflows.
// However, that is fine here (it would still represent the full range),
// i.e., if the range is everything.
let bits = self.value.size(cx).bits();
assert!(bits <= 128);
let mask = !0u128 >> (128 - bits);
let start = self.valid_range.start;
let end = self.valid_range.end.wrapping_add(1);
if start & mask == end & mask {
// The lo==hi case would be rejected by the LLVM verifier
// (it would mean either an empty set, which is impossible, or the
// entire range of the type, which is pointless).
return None;
}
Some(start..end)
}
}

/// The first half of a fat pointer.
Expand Down
19 changes: 18 additions & 1 deletion src/librustc_trans/abi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1042,7 +1042,7 @@ impl<'a, 'tcx> FnType<'tcx> {
}
}

pub fn apply_attrs_callsite(&self, callsite: ValueRef) {
pub fn apply_attrs_callsite(&self, bx: &Builder<'a, 'tcx>, callsite: ValueRef) {
let mut i = 0;
let mut apply = |attrs: &ArgAttributes| {
attrs.apply_callsite(llvm::AttributePlace::Argument(i), callsite);
Expand All @@ -1055,6 +1055,23 @@ impl<'a, 'tcx> FnType<'tcx> {
PassMode::Indirect(ref attrs) => apply(attrs),
_ => {}
}
if let layout::Abi::Scalar(ref 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.
match scalar.value {
layout::Int(..) if !scalar.is_bool() => {
if let Some(range) = scalar.range_metadata(bx.cx) {
// FIXME(nox): This causes very weird type errors about
// SHL operators in constants in stage 2 with LLVM 3.9.
if unsafe { llvm::LLVMRustVersionMajor() >= 4 } {
bx.range_metadata(callsite, range);
}
}
}
_ => {}
}
}
for arg in &self.args {
if arg.pad.is_some() {
apply(&ArgAttributes::new());
Expand Down
1 change: 1 addition & 0 deletions src/librustc_trans/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
#![allow(unused_attributes)]
#![feature(libc)]
#![feature(quote)]
#![feature(range_contains)]
#![feature(rustc_diagnostic_macros)]
#![feature(slice_sort_by_cached_key)]
#![feature(optin_builtin_traits)]
Expand Down
4 changes: 2 additions & 2 deletions src/librustc_trans/mir/block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ impl<'a, 'tcx> FunctionCx<'a, 'tcx> {
ret_bx,
llblock(this, cleanup),
cleanup_bundle);
fn_ty.apply_attrs_callsite(invokeret);
fn_ty.apply_attrs_callsite(&bx, invokeret);

if let Some((ret_dest, target)) = destination {
let ret_bx = this.build_block(target);
Expand All @@ -136,7 +136,7 @@ impl<'a, 'tcx> FunctionCx<'a, 'tcx> {
}
} else {
let llret = bx.call(fn_ptr, &llargs, cleanup_bundle);
fn_ty.apply_attrs_callsite(llret);
fn_ty.apply_attrs_callsite(&bx, llret);
if this.mir[bb].is_cleanup {
// Cleanup is always the cold path. Don't inline
// drop glue. Also, when there is a deeply-nested
Expand Down
22 changes: 6 additions & 16 deletions src/librustc_trans/mir/place.rs
Original file line number Diff line number Diff line change
Expand Up @@ -91,24 +91,14 @@ impl<'a, 'tcx> PlaceRef<'tcx> {
}

let scalar_load_metadata = |load, scalar: &layout::Scalar| {
let (min, max) = (scalar.valid_range.start, scalar.valid_range.end);
let max_next = max.wrapping_add(1);
let bits = scalar.value.size(bx.cx).bits();
assert!(bits <= 128);
let mask = !0u128 >> (128 - bits);
// For a (max) value of -1, max will be `-1 as usize`, which overflows.
// However, that is fine here (it would still represent the full range),
// i.e., if the range is everything. The lo==hi case would be
// rejected by the LLVM verifier (it would mean either an
// empty set, which is impossible, or the entire range of the
// type, which is pointless).
match scalar.value {
layout::Int(..) if max_next & mask != min & mask => {
// llvm::ConstantRange can deal with ranges that wrap around,
// so an overflow on (max + 1) is fine.
bx.range_metadata(load, min..max_next);
layout::Int(..) => {
if let Some(range) = scalar.range_metadata(bx.cx) {
bx.range_metadata(load, range);
}
}
layout::Pointer if 0 < min && min < max => {
layout::Pointer
if (1..scalar.valid_range.end).contains(&scalar.valid_range.start) => {
bx.nonnull_metadata(load);
}
_ => {}
Expand Down
29 changes: 29 additions & 0 deletions src/test/codegen/call-metadata.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
// Copyright 2016 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.

// Checks that range metadata gets emitted on calls to functions returning a
// scalar value.

// compile-flags: -C no-prepopulate-passes
// min-llvm-version 4.0


#![crate_type = "lib"]

pub fn test() {
// CHECK: call i8 @some_true(), !range [[R0:![0-9]+]]
// CHECK: [[R0]] = !{i8 0, i8 3}
some_true();
}

#[no_mangle]
fn some_true() -> Option<bool> {
Some(true)
}

0 comments on commit c4ba253

Please sign in to comment.