diff options
| author | Nikita Popov <nikita.ppv@gmail.com> | 2021-03-18 21:50:28 +0100 |
|---|---|---|
| committer | Nikita Popov <nikita.ppv@gmail.com> | 2021-03-21 20:10:53 +0100 |
| commit | dfc4cafe8e7b5c5198ba3a9d9c57a2e1f09d03bd (patch) | |
| tree | 046a10c5a9fdce4babdfa73e18f9ce442e138c51 | |
| parent | f82664191d0e8764b7435b9d72eb0e366b8b1464 (diff) | |
| download | rust-dfc4cafe8e7b5c5198ba3a9d9c57a2e1f09d03bd.tar.gz rust-dfc4cafe8e7b5c5198ba3a9d9c57a2e1f09d03bd.zip | |
Move decision aboute noalias into codegen_llvm
The frontend shouldn't be deciding whether or not to use mutable noalias attributes, as this is a pure LLVM concern. Only provide the necessary information and do the actual decision in codegen_llvm.
| -rw-r--r-- | compiler/rustc_codegen_llvm/src/abi.rs | 67 | ||||
| -rw-r--r-- | compiler/rustc_middle/src/ty/layout.rs | 31 | ||||
| -rw-r--r-- | compiler/rustc_target/src/abi/call/mod.rs | 5 | ||||
| -rw-r--r-- | compiler/rustc_target/src/abi/mod.rs | 2 |
4 files changed, 65 insertions, 40 deletions
diff --git a/compiler/rustc_codegen_llvm/src/abi.rs b/compiler/rustc_codegen_llvm/src/abi.rs index d9393ffe534..46e8e2a71cb 100644 --- a/compiler/rustc_codegen_llvm/src/abi.rs +++ b/compiler/rustc_codegen_llvm/src/abi.rs @@ -41,12 +41,32 @@ impl ArgAttributeExt for ArgAttribute { } pub trait ArgAttributesExt { - fn apply_attrs_to_llfn(&self, idx: AttributePlace, llfn: &Value); - fn apply_attrs_to_callsite(&self, idx: AttributePlace, callsite: &Value); + fn apply_attrs_to_llfn(&self, idx: AttributePlace, cx: &CodegenCx<'_, '_>, llfn: &Value); + fn apply_attrs_to_callsite( + &self, + idx: AttributePlace, + cx: &CodegenCx<'_, '_>, + callsite: &Value, + ); +} + +fn should_use_mutable_noalias(cx: &CodegenCx<'_, '_>) -> bool { + // Previously we would only emit noalias annotations for LLVM >= 6 or in + // panic=abort mode. That was deemed right, as prior versions had many bugs + // in conjunction with unwinding, but later versions didn’t seem to have + // said issues. See issue #31681. + // + // Alas, later on we encountered a case where noalias would generate wrong + // code altogether even with recent versions of LLVM in *safe* code with no + // unwinding involved. See #54462. + // + // For now, do not enable mutable_noalias by default at all, while the + // issue is being figured out. + cx.tcx.sess.opts.debugging_opts.mutable_noalias } impl ArgAttributesExt for ArgAttributes { - fn apply_attrs_to_llfn(&self, idx: AttributePlace, llfn: &Value) { + fn apply_attrs_to_llfn(&self, idx: AttributePlace, cx: &CodegenCx<'_, '_>, llfn: &Value) { let mut regular = self.regular; unsafe { let deref = self.pointee_size.bytes(); @@ -62,6 +82,9 @@ impl ArgAttributesExt for ArgAttributes { llvm::LLVMRustAddAlignmentAttr(llfn, idx.as_uint(), align.bytes() as u32); } regular.for_each_kind(|attr| attr.apply_llfn(idx, llfn)); + if regular.contains(ArgAttribute::NoAliasMutRef) && should_use_mutable_noalias(cx) { + llvm::Attribute::NoAlias.apply_llfn(idx, llfn); + } match self.arg_ext { ArgExtension::None => {} ArgExtension::Zext => { @@ -74,7 +97,12 @@ impl ArgAttributesExt for ArgAttributes { } } - fn apply_attrs_to_callsite(&self, idx: AttributePlace, callsite: &Value) { + fn apply_attrs_to_callsite( + &self, + idx: AttributePlace, + cx: &CodegenCx<'_, '_>, + callsite: &Value, + ) { let mut regular = self.regular; unsafe { let deref = self.pointee_size.bytes(); @@ -98,6 +126,9 @@ impl ArgAttributesExt for ArgAttributes { ); } regular.for_each_kind(|attr| attr.apply_callsite(idx, callsite)); + if regular.contains(ArgAttribute::NoAliasMutRef) && should_use_mutable_noalias(cx) { + llvm::Attribute::NoAlias.apply_callsite(idx, callsite); + } match self.arg_ext { ArgExtension::None => {} ArgExtension::Zext => { @@ -419,13 +450,13 @@ impl<'tcx> FnAbiLlvmExt<'tcx> for FnAbi<'tcx, Ty<'tcx>> { let mut i = 0; let mut apply = |attrs: &ArgAttributes| { - attrs.apply_attrs_to_llfn(llvm::AttributePlace::Argument(i), llfn); + attrs.apply_attrs_to_llfn(llvm::AttributePlace::Argument(i), cx, llfn); i += 1; i - 1 }; match self.ret.mode { PassMode::Direct(ref attrs) => { - attrs.apply_attrs_to_llfn(llvm::AttributePlace::ReturnValue, llfn); + attrs.apply_attrs_to_llfn(llvm::AttributePlace::ReturnValue, cx, llfn); } PassMode::Indirect { ref attrs, extra_attrs: _, on_stack } => { assert!(!on_stack); @@ -480,18 +511,18 @@ impl<'tcx> FnAbiLlvmExt<'tcx> for FnAbi<'tcx, Ty<'tcx>> { // FIXME(wesleywiser, eddyb): We should apply `nounwind` and `noreturn` as appropriate to this callsite. let mut i = 0; - let mut apply = |attrs: &ArgAttributes| { - attrs.apply_attrs_to_callsite(llvm::AttributePlace::Argument(i), callsite); + let mut apply = |cx: &CodegenCx<'_, '_>, attrs: &ArgAttributes| { + attrs.apply_attrs_to_callsite(llvm::AttributePlace::Argument(i), cx, callsite); i += 1; i - 1 }; match self.ret.mode { PassMode::Direct(ref attrs) => { - attrs.apply_attrs_to_callsite(llvm::AttributePlace::ReturnValue, callsite); + attrs.apply_attrs_to_callsite(llvm::AttributePlace::ReturnValue, &bx.cx, callsite); } PassMode::Indirect { ref attrs, extra_attrs: _, on_stack } => { assert!(!on_stack); - let i = apply(attrs); + let i = apply(bx.cx, attrs); unsafe { llvm::LLVMRustAddStructRetCallSiteAttr( callsite, @@ -517,12 +548,12 @@ impl<'tcx> FnAbiLlvmExt<'tcx> for FnAbi<'tcx, Ty<'tcx>> { } for arg in &self.args { if arg.pad.is_some() { - apply(&ArgAttributes::new()); + apply(bx.cx, &ArgAttributes::new()); } match arg.mode { PassMode::Ignore => {} PassMode::Indirect { ref attrs, extra_attrs: None, on_stack: true } => { - let i = apply(attrs); + let i = apply(bx.cx, attrs); unsafe { llvm::LLVMRustAddByValCallSiteAttr( callsite, @@ -533,22 +564,22 @@ impl<'tcx> FnAbiLlvmExt<'tcx> for FnAbi<'tcx, Ty<'tcx>> { } PassMode::Direct(ref attrs) | PassMode::Indirect { ref attrs, extra_attrs: None, on_stack: false } => { - apply(attrs); + apply(bx.cx, attrs); } PassMode::Indirect { ref attrs, extra_attrs: Some(ref extra_attrs), on_stack: _, } => { - apply(attrs); - apply(extra_attrs); + apply(bx.cx, attrs); + apply(bx.cx, extra_attrs); } PassMode::Pair(ref a, ref b) => { - apply(a); - apply(b); + apply(bx.cx, a); + apply(bx.cx, b); } PassMode::Cast(_) => { - apply(&ArgAttributes::new()); + apply(bx.cx, &ArgAttributes::new()); } } } diff --git a/compiler/rustc_middle/src/ty/layout.rs b/compiler/rustc_middle/src/ty/layout.rs index 814581a6cf1..0ddc1093a48 100644 --- a/compiler/rustc_middle/src/ty/layout.rs +++ b/compiler/rustc_middle/src/ty/layout.rs @@ -2327,24 +2327,7 @@ where PointerKind::Shared } } - hir::Mutability::Mut => { - // Previously we would only emit noalias annotations for LLVM >= 6 or in - // panic=abort mode. That was deemed right, as prior versions had many bugs - // in conjunction with unwinding, but later versions didn’t seem to have - // said issues. See issue #31681. - // - // Alas, later on we encountered a case where noalias would generate wrong - // code altogether even with recent versions of LLVM in *safe* code with no - // unwinding involved. See #54462. - // - // For now, do not enable mutable_noalias by default at all, while the - // issue is being figured out. - if tcx.sess.opts.debugging_opts.mutable_noalias { - PointerKind::UniqueBorrowed - } else { - PointerKind::Shared - } - } + hir::Mutability::Mut => PointerKind::UniqueBorrowed, }; cx.layout_of(ty).to_result().ok().map(|layout| PointeeInfo { @@ -2775,10 +2758,14 @@ where // and can be marked as both `readonly` and `noalias`, as // LLVM's definition of `noalias` is based solely on memory // dependencies rather than pointer equality + // + // Due to miscompiles in LLVM < 12, we apply a separate NoAliasMutRef attribute + // for UniqueBorrowed arguments, so that the codegen backend can decide + // whether or not to actually emit the attribute. let no_alias = match kind { - PointerKind::Shared => false, + PointerKind::Shared | PointerKind::UniqueBorrowed => false, PointerKind::UniqueOwned => true, - PointerKind::Frozen | PointerKind::UniqueBorrowed => !is_return, + PointerKind::Frozen => !is_return, }; if no_alias { attrs.set(ArgAttribute::NoAlias); @@ -2787,6 +2774,10 @@ where if kind == PointerKind::Frozen && !is_return { attrs.set(ArgAttribute::ReadOnly); } + + if kind == PointerKind::UniqueBorrowed && !is_return { + attrs.set(ArgAttribute::NoAliasMutRef); + } } } }; diff --git a/compiler/rustc_target/src/abi/call/mod.rs b/compiler/rustc_target/src/abi/call/mod.rs index 0deb1186b0f..2c3f7762759 100644 --- a/compiler/rustc_target/src/abi/call/mod.rs +++ b/compiler/rustc_target/src/abi/call/mod.rs @@ -65,7 +65,10 @@ mod attr_impl { const NoCapture = 1 << 2; const NonNull = 1 << 3; const ReadOnly = 1 << 4; - const InReg = 1 << 8; + const InReg = 1 << 5; + // NoAlias on &mut arguments can only be used with LLVM >= 12 due to miscompiles + // in earlier versions. FIXME: Remove this distinction once possible. + const NoAliasMutRef = 1 << 6; } } } diff --git a/compiler/rustc_target/src/abi/mod.rs b/compiler/rustc_target/src/abi/mod.rs index b14b1ef00db..e2618da749f 100644 --- a/compiler/rustc_target/src/abi/mod.rs +++ b/compiler/rustc_target/src/abi/mod.rs @@ -1112,7 +1112,7 @@ pub enum PointerKind { /// `&T` where `T` contains no `UnsafeCell`, is `noalias` and `readonly`. Frozen, - /// `&mut T`, when we know `noalias` is safe for LLVM. + /// `&mut T` which is `noalias` but not `readonly`. UniqueBorrowed, /// `Box<T>`, unlike `UniqueBorrowed`, it also has `noalias` on returns. |
