Auto merge of #50648 - nox:volatile-store, r=eddyb

Fix volatile_store and nontemporal_store

Fixes #50371.

9 files changed, 75 insertions, 81 deletions

diff --git a/src/Cargo.lock b/src/Cargo.lock
index 4cb82f94251..70a230cb7f7 100644
--- a/src/Cargo.lock
+++ b/src/Cargo.lock
@@ -2178,6 +2178,7 @@ dependencies = [
 name = "rustc_trans"
 version = "0.0.0"
 dependencies = [
+ "bitflags 1.0.1 (registry+https://github.com/rust-lang/crates.io-index)",
  "cc 1.0.15 (registry+https://github.com/rust-lang/crates.io-index)",
  "env_logger 0.5.8 (registry+https://github.com/rust-lang/crates.io-index)",
  "flate2 1.0.1 (registry+https://github.com/rust-lang/crates.io-index)",
diff --git a/src/librustc_trans/Cargo.toml b/src/librustc_trans/Cargo.toml
index a4dd02e97b2..64d3a4f4d53 100644
--- a/src/librustc_trans/Cargo.toml
+++ b/src/librustc_trans/Cargo.toml
@@ -10,6 +10,7 @@ crate-type = ["dylib"]
 test = false
 
 [dependencies]
+bitflags = "1.0.1"
 cc = "1.0.1"
 flate2 = "1.0"
 jobserver = "0.1.5"
diff --git a/src/librustc_trans/abi.rs b/src/librustc_trans/abi.rs
index 1838dae049a..25c598c532c 100644
--- a/src/librustc_trans/abi.rs
+++ b/src/librustc_trans/abi.rs
@@ -10,7 +10,7 @@
 
 use llvm::{self, ValueRef, AttributePlace};
 use base;
-use builder::Builder;
+use builder::{Builder, MemFlags};
 use common::{ty_fn_sig, C_usize};
 use context::CodegenCx;
 use mir::place::PlaceRef;
@@ -220,7 +220,8 @@ impl<'a, 'tcx> ArgTypeExt<'a, 'tcx> for ArgType<'tcx, Ty<'tcx>> {
                                   bx.pointercast(dst.llval, Type::i8p(cx)),
                                   bx.pointercast(llscratch, Type::i8p(cx)),
                                   C_usize(cx, self.layout.size.bytes()),
-                                  self.layout.align.min(scratch_align));
+                                  self.layout.align.min(scratch_align),
+                                  MemFlags::empty());
 
                 bx.lifetime_end(llscratch, scratch_size);
             }
diff --git a/src/librustc_trans/base.rs b/src/librustc_trans/base.rs
index 0dd1adbff86..feca36fa6c2 100644
--- a/src/librustc_trans/base.rs
+++ b/src/librustc_trans/base.rs
@@ -53,7 +53,7 @@ use rustc_incremental;
 use allocator;
 use mir::place::PlaceRef;
 use attributes;
-use builder::Builder;
+use builder::{Builder, MemFlags};
 use callee;
 use common::{C_bool, C_bytes_in_context, C_i32, C_usize};
 use rustc_mir::monomorphize::collector::{self, MonoItemCollectionMode};
@@ -320,7 +320,7 @@ pub fn coerce_unsized_into<'a, 'tcx>(bx: &Builder<'a, 'tcx>,
 
                 if src_f.layout.ty == dst_f.layout.ty {
                     memcpy_ty(bx, dst_f.llval, src_f.llval, src_f.layout,
-                        src_f.align.min(dst_f.align));
+                              src_f.align.min(dst_f.align), MemFlags::empty());
                 } else {
                     coerce_unsized_into(bx, src_f, dst_f);
                 }
@@ -408,7 +408,15 @@ pub fn call_memcpy(bx: &Builder,
                    dst: ValueRef,
                    src: ValueRef,
                    n_bytes: ValueRef,
-                   align: Align) {
+                   align: Align,
+                   flags: MemFlags) {
+    if flags.contains(MemFlags::NONTEMPORAL) {
+        // HACK(nox): This is inefficient but there is no nontemporal memcpy.
+        let val = bx.load(src, align);
+        let ptr = bx.pointercast(dst, val_ty(val).ptr_to());
+        bx.store_with_flags(val, ptr, align, flags);
+        return;
+    }
     let cx = bx.cx;
     let ptr_width = &cx.sess().target.target.target_pointer_width;
     let key = format!("llvm.memcpy.p0i8.p0i8.i{}", ptr_width);
@@ -417,7 +425,7 @@ pub fn call_memcpy(bx: &Builder,
     let dst_ptr = bx.pointercast(dst, Type::i8p(cx));
     let size = bx.intcast(n_bytes, cx.isize_ty, false);
     let align = C_i32(cx, align.abi() as i32);
-    let volatile = C_bool(cx, false);
+    let volatile = C_bool(cx, flags.contains(MemFlags::VOLATILE));
     bx.call(memcpy, &[dst_ptr, src_ptr, size, align, volatile], None);
 }
 
@@ -427,13 +435,14 @@ pub fn memcpy_ty<'a, 'tcx>(
     src: ValueRef,
     layout: TyLayout<'tcx>,
     align: Align,
+    flags: MemFlags,
 ) {
     let size = layout.size.bytes();
     if size == 0 {
         return;
     }
 
-    call_memcpy(bx, dst, src, C_usize(bx.cx, size), align);
+    call_memcpy(bx, dst, src, C_usize(bx.cx, size), align, flags);
 }
 
 pub fn call_memset<'a, 'tcx>(bx: &Builder<'a, 'tcx>,
diff --git a/src/librustc_trans/builder.rs b/src/librustc_trans/builder.rs
index db803ca8209..4153c61e526 100644
--- a/src/librustc_trans/builder.rs
+++ b/src/librustc_trans/builder.rs
@@ -50,6 +50,13 @@ fn noname() -> *const c_char {
     &CNULL
 }
 
+bitflags! {
+    pub struct MemFlags: u8 {
+        const VOLATILE = 1 << 0;
+        const NONTEMPORAL = 1 << 1;
+    }
+}
+
 impl<'a, 'tcx> Builder<'a, 'tcx> {
     pub fn new_block<'b>(cx: &'a CodegenCx<'a, 'tcx>, llfn: ValueRef, name: &'b str) -> Self {
         let bx = Builder::with_cx(cx);
@@ -579,29 +586,39 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
     }
 
     pub fn store(&self, val: ValueRef, ptr: ValueRef, align: Align) -> ValueRef {
-        debug!("Store {:?} -> {:?}", Value(val), Value(ptr));
+        self.store_with_flags(val, ptr, align, MemFlags::empty())
+    }
+
+    pub fn store_with_flags(
+        &self,
+        val: ValueRef,
+        ptr: ValueRef,
+        align: Align,
+        flags: MemFlags,
+    ) -> ValueRef {
+        debug!("Store {:?} -> {:?} ({:?})", Value(val), Value(ptr), flags);
         assert!(!self.llbuilder.is_null());
         self.count_insn("store");
         let ptr = self.check_store(val, ptr);
         unsafe {
             let store = llvm::LLVMBuildStore(self.llbuilder, val, ptr);
             llvm::LLVMSetAlignment(store, align.abi() as c_uint);
+            if flags.contains(MemFlags::VOLATILE) {
+                llvm::LLVMSetVolatile(store, llvm::True);
+            }
+            if flags.contains(MemFlags::NONTEMPORAL) {
+                // According to LLVM [1] building a nontemporal store must
+                // *always* point to a metadata value of the integer 1.
+                //
+                // [1]: http://llvm.org/docs/LangRef.html#store-instruction
+                let one = C_i32(self.cx, 1);
+                let node = llvm::LLVMMDNodeInContext(self.cx.llcx, &one, 1);
+                llvm::LLVMSetMetadata(store, llvm::MD_nontemporal as c_uint, node);
+            }
             store
         }
     }
 
-    pub fn volatile_store(&self, val: ValueRef, ptr: ValueRef) -> ValueRef {
-        debug!("Store {:?} -> {:?}", Value(val), Value(ptr));
-        assert!(!self.llbuilder.is_null());
-        self.count_insn("store.volatile");
-        let ptr = self.check_store(val, ptr);
-        unsafe {
-            let insn = llvm::LLVMBuildStore(self.llbuilder, val, ptr);
-            llvm::LLVMSetVolatile(insn, llvm::True);
-            insn
-        }
-    }
-
     pub fn atomic_store(&self, val: ValueRef, ptr: ValueRef,
                         order: AtomicOrdering, align: Align) {
         debug!("Store {:?} -> {:?}", Value(val), Value(ptr));
@@ -615,29 +632,6 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
         }
     }
 
-    pub fn nontemporal_store(&self, val: ValueRef, ptr: ValueRef) -> ValueRef {
-        debug!("Store {:?} -> {:?}", Value(val), Value(ptr));
-        assert!(!self.llbuilder.is_null());
-        self.count_insn("store.nontemporal");
-        let ptr = self.check_store(val, ptr);
-        unsafe {
-            let insn = llvm::LLVMBuildStore(self.llbuilder, val, ptr);
-
-            // According to LLVM [1] building a nontemporal store must *always*
-            // point to a metadata value of the integer 1. Who knew?
-            //
-            // [1]: http://llvm.org/docs/LangRef.html#store-instruction
-            let one = C_i32(self.cx, 1);
-            let node = llvm::LLVMMDNodeInContext(self.cx.llcx,
-                                                 &one,
-                                                 1);
-            llvm::LLVMSetMetadata(insn,
-                                  llvm::MD_nontemporal as c_uint,
-                                  node);
-            insn
-        }
-    }
-
     pub fn gep(&self, ptr: ValueRef, indices: &[ValueRef]) -> ValueRef {
         self.count_insn("gep");
         unsafe {
diff --git a/src/librustc_trans/intrinsic.rs b/src/librustc_trans/intrinsic.rs
index 49a207a2d8a..86aa48b6a9e 100644
--- a/src/librustc_trans/intrinsic.rs
+++ b/src/librustc_trans/intrinsic.rs
@@ -247,26 +247,8 @@ pub fn trans_intrinsic_call<'a, 'tcx>(bx: &Builder<'a, 'tcx>,
             to_immediate(bx, load, cx.layout_of(tp_ty))
         },
         "volatile_store" => {
-            let tp_ty = substs.type_at(0);
             let dst = args[0].deref(bx.cx);
-            if let OperandValue::Pair(a, b) = args[1].val {
-                bx.volatile_store(a, dst.project_field(bx, 0).llval);
-                bx.volatile_store(b, dst.project_field(bx, 1).llval);
-            } else {
-                let val = if let OperandValue::Ref(ptr, align) = args[1].val {
-                    bx.load(ptr, align)
-                } else {
-                    if dst.layout.is_zst() {
-                        return;
-                    }
-                    from_immediate(bx, args[1].immediate())
-                };
-                let ptr = bx.pointercast(dst.llval, val_ty(val).ptr_to());
-                let store = bx.volatile_store(val, ptr);
-                unsafe {
-                    llvm::LLVMSetAlignment(store, cx.align_of(tp_ty).abi() as u32);
-                }
-            }
+            args[1].val.volatile_store(bx, dst);
             return;
         },
         "prefetch_read_data" | "prefetch_write_data" |
@@ -551,19 +533,9 @@ pub fn trans_intrinsic_call<'a, 'tcx>(bx: &Builder<'a, 'tcx>,
         }
 
         "nontemporal_store" => {
-            let tp_ty = substs.type_at(0);
             let dst = args[0].deref(bx.cx);
-            let val = if let OperandValue::Ref(ptr, align) = args[1].val {
-                bx.load(ptr, align)
-            } else {
-                from_immediate(bx, args[1].immediate())
-            };
-            let ptr = bx.pointercast(dst.llval, val_ty(val).ptr_to());
-            let store = bx.nontemporal_store(val, ptr);
-            unsafe {
-                llvm::LLVMSetAlignment(store, cx.align_of(tp_ty).abi() as u32);
-            }
-            return
+            args[1].val.nontemporal_store(bx, dst);
+            return;
         }
 
         _ => {
diff --git a/src/librustc_trans/lib.rs b/src/librustc_trans/lib.rs
index 30780b8c965..6db95657ce0 100644
--- a/src/librustc_trans/lib.rs
+++ b/src/librustc_trans/lib.rs
@@ -34,6 +34,7 @@
 use rustc::dep_graph::WorkProduct;
 use syntax_pos::symbol::Symbol;
 
+#[macro_use] extern crate bitflags;
 extern crate flate2;
 extern crate libc;
 #[macro_use] extern crate rustc;
diff --git a/src/librustc_trans/mir/block.rs b/src/librustc_trans/mir/block.rs
index b666c2b2115..e4989da36c0 100644
--- a/src/librustc_trans/mir/block.rs
+++ b/src/librustc_trans/mir/block.rs
@@ -17,7 +17,7 @@ use rustc::mir::interpret::EvalErrorKind;
 use abi::{Abi, ArgType, ArgTypeExt, FnType, FnTypeExt, LlvmType, PassMode};
 use base;
 use callee;
-use builder::Builder;
+use builder::{Builder, MemFlags};
 use common::{self, C_bool, C_str_slice, C_struct, C_u32, C_uint_big, C_undef};
 use consts;
 use meth;
@@ -626,7 +626,7 @@ impl<'a, 'tcx> FunctionCx<'a, 'tcx> {
                     // have scary latent bugs around.
 
                     let scratch = PlaceRef::alloca(bx, arg.layout, "arg");
-                    base::memcpy_ty(bx, scratch.llval, llval, op.layout, align);
+                    base::memcpy_ty(bx, scratch.llval, llval, op.layout, align, MemFlags::empty());
                     (scratch.llval, scratch.align, true)
                 } else {
                     (llval, align, true)
diff --git a/src/librustc_trans/mir/operand.rs b/src/librustc_trans/mir/operand.rs
index d2bf8394296..be14da1a195 100644
--- a/src/librustc_trans/mir/operand.rs
+++ b/src/librustc_trans/mir/operand.rs
@@ -18,7 +18,7 @@ use rustc_data_structures::indexed_vec::Idx;
 
 use base;
 use common::{self, CodegenCx, C_null, C_undef, C_usize};
-use builder::Builder;
+use builder::{Builder, MemFlags};
 use value::Value;
 use type_of::LayoutLlvmExt;
 use type_::Type;
@@ -272,6 +272,18 @@ impl<'a, 'tcx> OperandRef<'tcx> {
 
 impl<'a, 'tcx> OperandValue {
     pub fn store(self, bx: &Builder<'a, 'tcx>, dest: PlaceRef<'tcx>) {
+        self.store_with_flags(bx, dest, MemFlags::empty());
+    }
+
+    pub fn volatile_store(self, bx: &Builder<'a, 'tcx>, dest: PlaceRef<'tcx>) {
+        self.store_with_flags(bx, dest, MemFlags::VOLATILE);
+    }
+
+    pub fn nontemporal_store(self, bx: &Builder<'a, 'tcx>, dest: PlaceRef<'tcx>) {
+        self.store_with_flags(bx, dest, MemFlags::NONTEMPORAL);
+    }
+
+    fn store_with_flags(self, bx: &Builder<'a, 'tcx>, dest: PlaceRef<'tcx>, flags: MemFlags) {
         debug!("OperandRef::store: operand={:?}, dest={:?}", self, dest);
         // Avoid generating stores of zero-sized values, because the only way to have a zero-sized
         // value is through `undef`, and store itself is useless.
@@ -279,11 +291,13 @@ impl<'a, 'tcx> OperandValue {
             return;
         }
         match self {
-            OperandValue::Ref(r, source_align) =>
+            OperandValue::Ref(r, source_align) => {
                 base::memcpy_ty(bx, dest.llval, r, dest.layout,
-                                source_align.min(dest.align)),
+                                source_align.min(dest.align), flags)
+            }
             OperandValue::Immediate(s) => {
-                bx.store(base::from_immediate(bx, s), dest.llval, dest.align);
+                let val = base::from_immediate(bx, s);
+                bx.store_with_flags(val, dest.llval, dest.align, flags);
             }
             OperandValue::Pair(a, b) => {
                 for (i, &x) in [a, b].iter().enumerate() {
@@ -292,7 +306,8 @@ impl<'a, 'tcx> OperandValue {
                     if common::val_ty(x) == Type::i1(bx.cx) {
                         llptr = bx.pointercast(llptr, Type::i8p(bx.cx));
                     }
-                    bx.store(base::from_immediate(bx, x), llptr, dest.align);
+                    let val = base::from_immediate(bx, x);
+                    bx.store_with_flags(val, llptr, dest.align, flags);
                 }
             }
         }