Auto merge of #50164 - nox:rval-range-metadata, r=eddyb

Emit range metadata on calls returning scalars (fixes #50157)

Author: bors

src/librustc_target/abi/mod.rs

@@ -14,7 +14,7 @@ pub use self::Primitive::*;
 use spec::Target;
 
 use std::cmp;
-use std::ops::{Add, Deref, Sub, Mul, AddAssign, RangeInclusive};
+use std::ops::{Add, Deref, Sub, Mul, AddAssign, Range, RangeInclusive};
 
 pub mod call;
 
@@ -544,6 +544,23 @@ impl Scalar {
             false
         }
     }
+
+    /// Returns the valid range as a `x..y` range.
+    ///
+    /// If `x` and `y` are equal, the range is full, not empty.
+    pub fn valid_range_exclusive<C: HasDataLayout>(&self, cx: C) -> 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;
+        assert_eq!(start, start & mask);
+        assert_eq!(end, end & mask);
+        start..(end.wrapping_add(1) & mask)
+    }
 }
 
 /// Describes how the fields of a type are located in memory.

src/librustc_target/lib.rs

@@ -29,6 +29,7 @@
 #![feature(const_fn)]
 #![feature(fs_read_write)]
 #![feature(inclusive_range)]
+#![feature(inclusive_range_fields)]
 #![feature(slice_patterns)]
 
 #[macro_use]

src/librustc_trans/abi.rs

@@ -265,7 +265,7 @@ pub trait FnTypeExt<'a, 'tcx> {
     fn llvm_type(&self, cx: &CodegenCx<'a, 'tcx>) -> Type;
     fn llvm_cconv(&self) -> llvm::CallConv;
     fn apply_attrs_llfn(&self, llfn: ValueRef);
-    fn apply_attrs_callsite(&self, callsite: ValueRef);
+    fn apply_attrs_callsite(&self, bx: &Builder<'a, 'tcx>, callsite: ValueRef);
 }
 
 impl<'a, 'tcx> FnTypeExt<'a, 'tcx> for FnType<'tcx, Ty<'tcx>> {
@@ -640,7 +640,7 @@ impl<'a, 'tcx> FnTypeExt<'a, 'tcx> for FnType<'tcx, Ty<'tcx>> {
         }
     }
 
-    fn apply_attrs_callsite(&self, callsite: ValueRef) {
+    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);
@@ -653,6 +653,24 @@ impl<'a, 'tcx> FnTypeExt<'a, 'tcx> for FnType<'tcx, Ty<'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() => {
+                    let range = scalar.valid_range_exclusive(bx.cx);
+                    if range.start != range.end {
+                        // 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());

src/librustc_trans/lib.rs

@@ -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)]

src/librustc_trans/mir/block.rs

@@ -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);
@@ -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

src/librustc_trans/mir/place.rs

@@ -91,24 +91,15 @@ 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).
+            let vr = scalar.valid_range.clone();
             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(..) => {
+                    let range = scalar.valid_range_exclusive(bx.cx);
+                    if range.start != range.end {
+                        bx.range_metadata(load, range);
+                    }
                 }
-                layout::Pointer if 0 < min && min < max => {
+                layout::Pointer if vr.start < vr.end && !vr.contains(&0) => {
                     bx.nonnull_metadata(load);
                 }
                 _ => {}

src/test/codegen/call-metadata.rs

@@ -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)
+}