Skip to content

Commit

Permalink
Rollup merge of #128679 - RalfJung:codegen-fn-attrs, r=nikic
Browse files Browse the repository at this point in the history
codegen: better centralize function declaration attribute computation

For some reason, the codegen backend has two functions that compute which attributes a function declaration gets: `apply_attrs_llfn` and `attributes::from_fn_attrs`. They are called in different places, on entirely different layers of abstraction.

To me the code seems cleaner if we centralize this entirely in `apply_attrs_llfn`, so that's what this PR does.
  • Loading branch information
matthiaskrgr authored Aug 7, 2024
2 parents e342295 + 273c67d commit 8f39b86
Show file tree
Hide file tree
Showing 5 changed files with 28 additions and 11 deletions.
26 changes: 23 additions & 3 deletions compiler/rustc_codegen_llvm/src/abi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,17 +5,18 @@ use rustc_codegen_ssa::mir::operand::{OperandRef, OperandValue};
use rustc_codegen_ssa::mir::place::{PlaceRef, PlaceValue};
use rustc_codegen_ssa::traits::*;
use rustc_codegen_ssa::MemFlags;
use rustc_middle::bug;
use rustc_middle::ty::layout::LayoutOf;
pub use rustc_middle::ty::layout::{FAT_PTR_ADDR, FAT_PTR_EXTRA};
use rustc_middle::ty::Ty;
use rustc_middle::{bug, ty};
use rustc_session::config;
pub use rustc_target::abi::call::*;
use rustc_target::abi::{self, HasDataLayout, Int, Size};
pub use rustc_target::spec::abi::Abi;
use rustc_target::spec::SanitizerSet;
use smallvec::SmallVec;

use crate::attributes::llfn_attrs_from_instance;
use crate::builder::Builder;
use crate::context::CodegenCx;
use crate::llvm::{self, Attribute, AttributePlace};
Expand Down Expand Up @@ -310,7 +311,16 @@ pub trait FnAbiLlvmExt<'ll, 'tcx> {
fn llvm_type(&self, cx: &CodegenCx<'ll, 'tcx>) -> &'ll Type;
fn ptr_to_llvm_type(&self, cx: &CodegenCx<'ll, 'tcx>) -> &'ll Type;
fn llvm_cconv(&self) -> llvm::CallConv;
fn apply_attrs_llfn(&self, cx: &CodegenCx<'ll, 'tcx>, llfn: &'ll Value);

/// Apply attributes to a function declaration/definition.
fn apply_attrs_llfn(
&self,
cx: &CodegenCx<'ll, 'tcx>,
llfn: &'ll Value,
instance: Option<ty::Instance<'tcx>>,
);

/// Apply attributes to a function call.
fn apply_attrs_callsite(&self, bx: &mut Builder<'_, 'll, 'tcx>, callsite: &'ll Value);
}

Expand Down Expand Up @@ -396,7 +406,12 @@ impl<'ll, 'tcx> FnAbiLlvmExt<'ll, 'tcx> for FnAbi<'tcx, Ty<'tcx>> {
self.conv.into()
}

fn apply_attrs_llfn(&self, cx: &CodegenCx<'ll, 'tcx>, llfn: &'ll Value) {
fn apply_attrs_llfn(
&self,
cx: &CodegenCx<'ll, 'tcx>,
llfn: &'ll Value,
instance: Option<ty::Instance<'tcx>>,
) {
let mut func_attrs = SmallVec::<[_; 3]>::new();
if self.ret.layout.abi.is_uninhabited() {
func_attrs.push(llvm::AttributeKind::NoReturn.create_attr(cx.llcx));
Expand Down Expand Up @@ -477,6 +492,11 @@ impl<'ll, 'tcx> FnAbiLlvmExt<'ll, 'tcx> for FnAbi<'tcx, Ty<'tcx>> {
}
}
}

// If the declaration has an associated instance, compute extra attributes based on that.
if let Some(instance) = instance {
llfn_attrs_from_instance(cx, llfn, instance);
}
}

fn apply_attrs_callsite(&self, bx: &mut Builder<'_, 'll, 'tcx>, callsite: &'ll Value) {
Expand Down
3 changes: 2 additions & 1 deletion compiler/rustc_codegen_llvm/src/attributes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -324,9 +324,10 @@ fn create_alloc_family_attr(llcx: &llvm::Context) -> &llvm::Attribute {
llvm::CreateAttrStringValue(llcx, "alloc-family", "__rust_alloc")
}

/// Helper for `FnAbi::apply_attrs_llfn`:
/// Composite function which sets LLVM attributes for function depending on its AST (`#[attribute]`)
/// attributes.
pub fn from_fn_attrs<'ll, 'tcx>(
pub fn llfn_attrs_from_instance<'ll, 'tcx>(
cx: &CodegenCx<'ll, 'tcx>,
llfn: &'ll Value,
instance: ty::Instance<'tcx>,
Expand Down
4 changes: 1 addition & 3 deletions compiler/rustc_codegen_llvm/src/callee.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@ use rustc_middle::ty::{self, Instance, TypeVisitableExt};
use tracing::debug;

use crate::context::CodegenCx;
use crate::llvm;
use crate::value::Value;
use crate::{attributes, llvm};

/// Codegens a reference to a fn/method item, monomorphizing and
/// inlining as it goes.
Expand Down Expand Up @@ -78,8 +78,6 @@ pub fn get_fn<'ll, 'tcx>(cx: &CodegenCx<'ll, 'tcx>, instance: Instance<'tcx>) ->
};
debug!("get_fn: not casting pointer!");

attributes::from_fn_attrs(cx, llfn, instance);

// Apply an appropriate linkage/visibility value to our item that we
// just declared.
//
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_codegen_llvm/src/declare.rs
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ impl<'ll, 'tcx> CodegenCx<'ll, 'tcx> {
llvm::Visibility::Default,
fn_abi.llvm_type(self),
);
fn_abi.apply_attrs_llfn(self, llfn);
fn_abi.apply_attrs_llfn(self, llfn, instance);

if self.tcx.sess.is_sanitizer_cfi_enabled() {
if let Some(instance) = instance {
Expand Down
4 changes: 1 addition & 3 deletions compiler/rustc_codegen_llvm/src/mono_item.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ use tracing::debug;
use crate::context::CodegenCx;
use crate::errors::SymbolAlreadyDefined;
use crate::type_of::LayoutLlvmExt;
use crate::{attributes, base, llvm};
use crate::{base, llvm};

impl<'tcx> PreDefineMethods<'tcx> for CodegenCx<'_, 'tcx> {
fn predefine_static(
Expand Down Expand Up @@ -87,8 +87,6 @@ impl<'tcx> PreDefineMethods<'tcx> for CodegenCx<'_, 'tcx> {

debug!("predefine_fn: instance = {:?}", instance);

attributes::from_fn_attrs(self, lldecl, instance);

unsafe {
if self.should_assume_dso_local(lldecl, false) {
llvm::LLVMRustSetDSOLocal(lldecl, true);
Expand Down

0 comments on commit 8f39b86

Please sign in to comment.