1
Fork 0

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.
This commit is contained in:
Nikita Popov 2021-03-18 21:50:28 +01:00
parent f82664191d
commit dfc4cafe8e
4 changed files with 65 additions and 40 deletions

View file

@ -41,12 +41,32 @@ impl ArgAttributeExt for ArgAttribute {
} }
pub trait ArgAttributesExt { pub trait ArgAttributesExt {
fn apply_attrs_to_llfn(&self, idx: AttributePlace, llfn: &Value); fn apply_attrs_to_llfn(&self, idx: AttributePlace, cx: &CodegenCx<'_, '_>, llfn: &Value);
fn apply_attrs_to_callsite(&self, idx: AttributePlace, callsite: &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 didnt 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 { 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; let mut regular = self.regular;
unsafe { unsafe {
let deref = self.pointee_size.bytes(); let deref = self.pointee_size.bytes();
@ -62,6 +82,9 @@ impl ArgAttributesExt for ArgAttributes {
llvm::LLVMRustAddAlignmentAttr(llfn, idx.as_uint(), align.bytes() as u32); llvm::LLVMRustAddAlignmentAttr(llfn, idx.as_uint(), align.bytes() as u32);
} }
regular.for_each_kind(|attr| attr.apply_llfn(idx, llfn)); 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 { match self.arg_ext {
ArgExtension::None => {} ArgExtension::None => {}
ArgExtension::Zext => { 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; let mut regular = self.regular;
unsafe { unsafe {
let deref = self.pointee_size.bytes(); let deref = self.pointee_size.bytes();
@ -98,6 +126,9 @@ impl ArgAttributesExt for ArgAttributes {
); );
} }
regular.for_each_kind(|attr| attr.apply_callsite(idx, callsite)); 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 { match self.arg_ext {
ArgExtension::None => {} ArgExtension::None => {}
ArgExtension::Zext => { ArgExtension::Zext => {
@ -419,13 +450,13 @@ impl<'tcx> FnAbiLlvmExt<'tcx> for FnAbi<'tcx, Ty<'tcx>> {
let mut i = 0; let mut i = 0;
let mut apply = |attrs: &ArgAttributes| { 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;
i - 1 i - 1
}; };
match self.ret.mode { match self.ret.mode {
PassMode::Direct(ref attrs) => { 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 } => { PassMode::Indirect { ref attrs, extra_attrs: _, on_stack } => {
assert!(!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. // FIXME(wesleywiser, eddyb): We should apply `nounwind` and `noreturn` as appropriate to this callsite.
let mut i = 0; let mut i = 0;
let mut apply = |attrs: &ArgAttributes| { let mut apply = |cx: &CodegenCx<'_, '_>, attrs: &ArgAttributes| {
attrs.apply_attrs_to_callsite(llvm::AttributePlace::Argument(i), callsite); attrs.apply_attrs_to_callsite(llvm::AttributePlace::Argument(i), cx, callsite);
i += 1; i += 1;
i - 1 i - 1
}; };
match self.ret.mode { match self.ret.mode {
PassMode::Direct(ref attrs) => { 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 } => { PassMode::Indirect { ref attrs, extra_attrs: _, on_stack } => {
assert!(!on_stack); assert!(!on_stack);
let i = apply(attrs); let i = apply(bx.cx, attrs);
unsafe { unsafe {
llvm::LLVMRustAddStructRetCallSiteAttr( llvm::LLVMRustAddStructRetCallSiteAttr(
callsite, callsite,
@ -517,12 +548,12 @@ impl<'tcx> FnAbiLlvmExt<'tcx> for FnAbi<'tcx, Ty<'tcx>> {
} }
for arg in &self.args { for arg in &self.args {
if arg.pad.is_some() { if arg.pad.is_some() {
apply(&ArgAttributes::new()); apply(bx.cx, &ArgAttributes::new());
} }
match arg.mode { match arg.mode {
PassMode::Ignore => {} PassMode::Ignore => {}
PassMode::Indirect { ref attrs, extra_attrs: None, on_stack: true } => { PassMode::Indirect { ref attrs, extra_attrs: None, on_stack: true } => {
let i = apply(attrs); let i = apply(bx.cx, attrs);
unsafe { unsafe {
llvm::LLVMRustAddByValCallSiteAttr( llvm::LLVMRustAddByValCallSiteAttr(
callsite, callsite,
@ -533,22 +564,22 @@ impl<'tcx> FnAbiLlvmExt<'tcx> for FnAbi<'tcx, Ty<'tcx>> {
} }
PassMode::Direct(ref attrs) PassMode::Direct(ref attrs)
| PassMode::Indirect { ref attrs, extra_attrs: None, on_stack: false } => { | PassMode::Indirect { ref attrs, extra_attrs: None, on_stack: false } => {
apply(attrs); apply(bx.cx, attrs);
} }
PassMode::Indirect { PassMode::Indirect {
ref attrs, ref attrs,
extra_attrs: Some(ref extra_attrs), extra_attrs: Some(ref extra_attrs),
on_stack: _, on_stack: _,
} => { } => {
apply(attrs); apply(bx.cx, attrs);
apply(extra_attrs); apply(bx.cx, extra_attrs);
} }
PassMode::Pair(ref a, ref b) => { PassMode::Pair(ref a, ref b) => {
apply(a); apply(bx.cx, a);
apply(b); apply(bx.cx, b);
} }
PassMode::Cast(_) => { PassMode::Cast(_) => {
apply(&ArgAttributes::new()); apply(bx.cx, &ArgAttributes::new());
} }
} }
} }

View file

@ -2327,24 +2327,7 @@ where
PointerKind::Shared PointerKind::Shared
} }
} }
hir::Mutability::Mut => { hir::Mutability::Mut => PointerKind::UniqueBorrowed,
// 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 didnt 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
}
}
}; };
cx.layout_of(ty).to_result().ok().map(|layout| PointeeInfo { 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 // and can be marked as both `readonly` and `noalias`, as
// LLVM's definition of `noalias` is based solely on memory // LLVM's definition of `noalias` is based solely on memory
// dependencies rather than pointer equality // 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 { let no_alias = match kind {
PointerKind::Shared => false, PointerKind::Shared | PointerKind::UniqueBorrowed => false,
PointerKind::UniqueOwned => true, PointerKind::UniqueOwned => true,
PointerKind::Frozen | PointerKind::UniqueBorrowed => !is_return, PointerKind::Frozen => !is_return,
}; };
if no_alias { if no_alias {
attrs.set(ArgAttribute::NoAlias); attrs.set(ArgAttribute::NoAlias);
@ -2787,6 +2774,10 @@ where
if kind == PointerKind::Frozen && !is_return { if kind == PointerKind::Frozen && !is_return {
attrs.set(ArgAttribute::ReadOnly); attrs.set(ArgAttribute::ReadOnly);
} }
if kind == PointerKind::UniqueBorrowed && !is_return {
attrs.set(ArgAttribute::NoAliasMutRef);
}
} }
} }
}; };

View file

@ -65,7 +65,10 @@ mod attr_impl {
const NoCapture = 1 << 2; const NoCapture = 1 << 2;
const NonNull = 1 << 3; const NonNull = 1 << 3;
const ReadOnly = 1 << 4; 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;
} }
} }
} }

View file

@ -1112,7 +1112,7 @@ pub enum PointerKind {
/// `&T` where `T` contains no `UnsafeCell`, is `noalias` and `readonly`. /// `&T` where `T` contains no `UnsafeCell`, is `noalias` and `readonly`.
Frozen, Frozen,
/// `&mut T`, when we know `noalias` is safe for LLVM. /// `&mut T` which is `noalias` but not `readonly`.
UniqueBorrowed, UniqueBorrowed,
/// `Box<T>`, unlike `UniqueBorrowed`, it also has `noalias` on returns. /// `Box<T>`, unlike `UniqueBorrowed`, it also has `noalias` on returns.