Skip to content

Commit

Permalink
Auto merge of #121644 - oli-obk:unique_static_innards2, r=<try>
Browse files Browse the repository at this point in the history
Ensure nested allocations in statics do not get deduplicated

This PR generates new `DefId`s for nested allocations in static items and feeds all the right queries to make the compiler believe these are regular `static` items. I chose this design, because all other designs are fragile and make the compiler horribly complex for such a niche use case.

At present this wrecks incremental compilation performance *in case nested allocations exist* (because any query creating a `DefId` will be recomputed and never loaded from the cache). This will be resolved later in #115613 . All other statics are unaffected by this change and will not have performance implications (heh, famous last words)

This PR contains various smaller refactorings that can be pulled out into separate PRs. It is best reviewed commit-by-commit. The last commit is where the actual magic happens.

r? `@RalfJung` on the const interner and engine changes
  • Loading branch information
bors committed Feb 26, 2024
2 parents b79db43 + 0aa7752 commit 51f4acc
Show file tree
Hide file tree
Showing 64 changed files with 388 additions and 164 deletions.
2 changes: 1 addition & 1 deletion compiler/rustc_ast_lowering/src/asm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
.get_partial_res(sym.id)
.and_then(|res| res.full_res())
.and_then(|res| match res {
Res::Def(DefKind::Static(_), def_id) => Some(def_id),
Res::Def(DefKind::Static { .. }, def_id) => Some(def_id),
_ => None,
});

Expand Down
6 changes: 3 additions & 3 deletions compiler/rustc_codegen_gcc/src/consts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ impl<'gcc, 'tcx> StaticMethods for CodegenCx<'gcc, 'tcx> {
global_value
}

fn codegen_static(&self, def_id: DefId, is_mutable: bool) {
fn codegen_static(&self, def_id: DefId) {
let attrs = self.tcx.codegen_fn_attrs(def_id);

let value =
Expand Down Expand Up @@ -89,7 +89,7 @@ impl<'gcc, 'tcx> StaticMethods for CodegenCx<'gcc, 'tcx> {

// As an optimization, all shared statics which do not have interior
// mutability are placed into read-only memory.
if !is_mutable {
if !self.tcx.static_mutability(def_id).unwrap().is_mut() {
if self.type_is_freeze(ty) {
#[cfg(feature = "master")]
global.global_set_readonly();
Expand Down Expand Up @@ -337,7 +337,7 @@ pub fn const_alloc_to_gcc<'gcc, 'tcx>(cx: &CodegenCx<'gcc, 'tcx>, alloc: ConstAl
cx.const_struct(&llvals, true)
}

pub fn codegen_static_initializer<'gcc, 'tcx>(cx: &CodegenCx<'gcc, 'tcx>, def_id: DefId) -> Result<(RValue<'gcc>, ConstAllocation<'tcx>), ErrorHandled> {
fn codegen_static_initializer<'gcc, 'tcx>(cx: &CodegenCx<'gcc, 'tcx>, def_id: DefId) -> Result<(RValue<'gcc>, ConstAllocation<'tcx>), ErrorHandled> {
let alloc = cx.tcx.eval_static_initializer(def_id)?;
Ok((const_alloc_to_gcc(cx, alloc), alloc))
}
Expand Down
6 changes: 5 additions & 1 deletion compiler/rustc_codegen_gcc/src/mono_item.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ use rustc_middle::middle::codegen_fn_attrs::CodegenFnAttrFlags;
use rustc_middle::mir::mono::{Linkage, Visibility};
use rustc_middle::ty::{self, Instance, TypeVisitableExt};
use rustc_middle::ty::layout::{FnAbiOf, LayoutOf};
use rustc_hir::def::DefKind;

use crate::attributes;
use crate::base;
Expand All @@ -17,7 +18,10 @@ impl<'gcc, 'tcx> PreDefineMethods<'tcx> for CodegenCx<'gcc, 'tcx> {
fn predefine_static(&self, def_id: DefId, _linkage: Linkage, visibility: Visibility, symbol_name: &str) {
let attrs = self.tcx.codegen_fn_attrs(def_id);
let instance = Instance::mono(self.tcx, def_id);
let ty = instance.ty(self.tcx, ty::ParamEnv::reveal_all());
let DefKind::Static { nested, .. } = self.tcx.def_kind(def_id) else { bug!() };
// Nested statics do not have a type, so pick a random type and let `define_static` figure out
// the llvm type from the actual evaluated initializer.
let ty = if nested { self.tcx.types.unit} else { instance.ty(self.tcx, ty::ParamEnv::reveal_all()) };
let gcc_type = self.layout_of(ty).gcc_type(self);

let is_tls = attrs.flags.contains(CodegenFnAttrFlags::THREAD_LOCAL);
Expand Down
104 changes: 63 additions & 41 deletions compiler/rustc_codegen_llvm/src/consts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ use crate::type_::Type;
use crate::type_of::LayoutLlvmExt;
use crate::value::Value;
use rustc_codegen_ssa::traits::*;
use rustc_hir::def::DefKind;
use rustc_hir::def_id::DefId;
use rustc_middle::middle::codegen_fn_attrs::{CodegenFnAttrFlags, CodegenFnAttrs};
use rustc_middle::mir::interpret::{
Expand All @@ -17,7 +18,7 @@ use rustc_middle::mir::interpret::{
};
use rustc_middle::mir::mono::MonoItem;
use rustc_middle::ty::layout::LayoutOf;
use rustc_middle::ty::{self, Instance, Ty};
use rustc_middle::ty::{self, Instance};
use rustc_middle::{bug, span_bug};
use rustc_session::config::Lto;
use rustc_target::abi::{
Expand Down Expand Up @@ -114,7 +115,7 @@ pub fn const_alloc_to_llvm<'ll>(cx: &CodegenCx<'ll, '_>, alloc: ConstAllocation<
cx.const_struct(&llvals, true)
}

pub fn codegen_static_initializer<'ll, 'tcx>(
fn codegen_static_initializer<'ll, 'tcx>(
cx: &CodegenCx<'ll, 'tcx>,
def_id: DefId,
) -> Result<(&'ll Value, ConstAllocation<'tcx>), ErrorHandled> {
Expand Down Expand Up @@ -147,11 +148,10 @@ fn set_global_alignment<'ll>(cx: &CodegenCx<'ll, '_>, gv: &'ll Value, mut align:
fn check_and_apply_linkage<'ll, 'tcx>(
cx: &CodegenCx<'ll, 'tcx>,
attrs: &CodegenFnAttrs,
ty: Ty<'tcx>,
llty: &'ll Type,
sym: &str,
def_id: DefId,
) -> &'ll Value {
let llty = cx.layout_of(ty).llvm_type(cx);
if let Some(linkage) = attrs.import_linkage {
debug!("get_static: sym={} linkage={:?}", sym, linkage);

Expand Down Expand Up @@ -226,9 +226,28 @@ impl<'ll> CodegenCx<'ll, '_> {
}
}

#[instrument(level = "debug", skip(self))]
pub(crate) fn get_static(&self, def_id: DefId) -> &'ll Value {
let instance = Instance::mono(self.tcx, def_id);
if let Some(&g) = self.instances.borrow().get(&instance) {
trace!(?instance);

let DefKind::Static { nested, .. } = self.tcx.def_kind(def_id) else { bug!() };
// Nested statics do not have a type, so pick a random type and let `define_static` figure out
// the llvm type from the actual evaluated initializer.
let llty = if nested {
self.type_i8()
} else {
let ty = instance.ty(self.tcx, ty::ParamEnv::reveal_all());
trace!(?ty);
self.layout_of(ty).llvm_type(self)
};
self.get_static_inner(def_id, llty)
}

#[instrument(level = "debug", skip(self, llty))]
pub(crate) fn get_static_inner(&self, def_id: DefId, llty: &'ll Type) -> &'ll Value {
if let Some(&g) = self.statics.borrow().get(&def_id) {
trace!("used cached value");
return g;
}

Expand All @@ -240,14 +259,12 @@ impl<'ll> CodegenCx<'ll, '_> {
statics defined in the same CGU, but did not for `{def_id:?}`"
);

let ty = instance.ty(self.tcx, ty::ParamEnv::reveal_all());
let sym = self.tcx.symbol_name(instance).name;
let sym = self.tcx.symbol_name(Instance::mono(self.tcx, def_id)).name;
let fn_attrs = self.tcx.codegen_fn_attrs(def_id);

debug!("get_static: sym={} instance={:?} fn_attrs={:?}", sym, instance, fn_attrs);
debug!(?sym, ?fn_attrs);

let g = if def_id.is_local() && !self.tcx.is_foreign_item(def_id) {
let llty = self.layout_of(ty).llvm_type(self);
if let Some(g) = self.get_declared_value(sym) {
if self.val_ty(g) != self.type_ptr() {
span_bug!(self.tcx.def_span(def_id), "Conflicting types for static");
Expand All @@ -264,7 +281,7 @@ impl<'ll> CodegenCx<'ll, '_> {

g
} else {
check_and_apply_linkage(self, fn_attrs, ty, sym, def_id)
check_and_apply_linkage(self, fn_attrs, llty, sym, def_id)
};

// Thread-local statics in some other crate need to *always* be linked
Expand Down Expand Up @@ -332,34 +349,15 @@ impl<'ll> CodegenCx<'ll, '_> {
}
}

self.instances.borrow_mut().insert(instance, g);
self.statics.borrow_mut().insert(def_id, g);
g
}
}

impl<'ll> StaticMethods for CodegenCx<'ll, '_> {
fn static_addr_of(&self, cv: &'ll Value, align: Align, kind: Option<&str>) -> &'ll Value {
if let Some(&gv) = self.const_globals.borrow().get(&cv) {
unsafe {
// Upgrade the alignment in cases where the same constant is used with different
// alignment requirements
let llalign = align.bytes() as u32;
if llalign > llvm::LLVMGetAlignment(gv) {
llvm::LLVMSetAlignment(gv, llalign);
}
}
return gv;
}
let gv = self.static_addr_of_mut(cv, align, kind);
unsafe {
llvm::LLVMSetGlobalConstant(gv, True);
}
self.const_globals.borrow_mut().insert(cv, gv);
gv
}

fn codegen_static(&self, def_id: DefId, is_mutable: bool) {
fn codegen_static_item(&self, def_id: DefId) {
unsafe {
assert!(
llvm::LLVMGetInitializer(self.statics.borrow().get(&def_id).unwrap()).is_none()
);
let attrs = self.tcx.codegen_fn_attrs(def_id);

let Ok((v, alloc)) = codegen_static_initializer(self, def_id) else {
Expand All @@ -368,13 +366,11 @@ impl<'ll> StaticMethods for CodegenCx<'ll, '_> {
};
let alloc = alloc.inner();

let g = self.get_static(def_id);

let val_llty = self.val_ty(v);

let instance = Instance::mono(self.tcx, def_id);
let ty = instance.ty(self.tcx, ty::ParamEnv::reveal_all());
let llty = self.layout_of(ty).llvm_type(self);
let g = self.get_static_inner(def_id, val_llty);
let llty = self.val_ty(g);

let g = if val_llty == llty {
g
} else {
Expand Down Expand Up @@ -409,7 +405,7 @@ impl<'ll> StaticMethods for CodegenCx<'ll, '_> {
self.statics_to_rauw.borrow_mut().push((g, new_g));
new_g
};
set_global_alignment(self, g, self.align_of(ty));
set_global_alignment(self, g, alloc.align);
llvm::LLVMSetInitializer(g, v);

if self.should_assume_dso_local(g, true) {
Expand All @@ -418,7 +414,7 @@ impl<'ll> StaticMethods for CodegenCx<'ll, '_> {

// As an optimization, all shared statics which do not have interior
// mutability are placed into read-only memory.
if !is_mutable && self.type_is_freeze(ty) {
if !self.tcx.is_mutable_static(def_id) && alloc.mutability.is_not() {
llvm::LLVMSetGlobalConstant(g, llvm::True);
}

Expand Down Expand Up @@ -541,6 +537,32 @@ impl<'ll> StaticMethods for CodegenCx<'ll, '_> {
}
}
}
}

impl<'ll> StaticMethods for CodegenCx<'ll, '_> {
fn static_addr_of(&self, cv: &'ll Value, align: Align, kind: Option<&str>) -> &'ll Value {
if let Some(&gv) = self.const_globals.borrow().get(&cv) {
unsafe {
// Upgrade the alignment in cases where the same constant is used with different
// alignment requirements
let llalign = align.bytes() as u32;
if llalign > llvm::LLVMGetAlignment(gv) {
llvm::LLVMSetAlignment(gv, llalign);
}
}
return gv;
}
let gv = self.static_addr_of_mut(cv, align, kind);
unsafe {
llvm::LLVMSetGlobalConstant(gv, True);
}
self.const_globals.borrow_mut().insert(cv, gv);
gv
}

fn codegen_static(&self, def_id: DefId) {
self.codegen_static_item(def_id)
}

/// Add a global value to a list to be stored in the `llvm.used` variable, an array of ptr.
fn add_used_global(&self, global: &'ll Value) {
Expand Down
4 changes: 4 additions & 0 deletions compiler/rustc_codegen_llvm/src/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,9 @@ pub struct CodegenCx<'ll, 'tcx> {

/// Cache instances of monomorphic and polymorphic items
pub instances: RefCell<FxHashMap<Instance<'tcx>, &'ll Value>>,

/// Cache static's allocations
pub statics: RefCell<FxHashMap<DefId, &'ll Value>>,
/// Cache generated vtables
pub vtables:
RefCell<FxHashMap<(Ty<'tcx>, Option<ty::PolyExistentialTraitRef<'tcx>>), &'ll Value>>,
Expand Down Expand Up @@ -467,6 +470,7 @@ impl<'ll, 'tcx> CodegenCx<'ll, 'tcx> {
llcx,
codegen_unit,
instances: Default::default(),
statics: Default::default(),
vtables: Default::default(),
const_str_cache: Default::default(),
const_globals: Default::default(),
Expand Down
13 changes: 11 additions & 2 deletions compiler/rustc_codegen_llvm/src/mono_item.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,9 @@ use crate::errors::SymbolAlreadyDefined;
use crate::llvm;
use crate::type_of::LayoutLlvmExt;
use rustc_codegen_ssa::traits::*;
use rustc_hir::def::DefKind;
use rustc_hir::def_id::{DefId, LOCAL_CRATE};
use rustc_middle::bug;
use rustc_middle::mir::mono::{Linkage, Visibility};
use rustc_middle::ty::layout::{FnAbiOf, LayoutOf};
use rustc_middle::ty::{self, Instance, TypeVisitableExt};
Expand All @@ -21,7 +23,14 @@ impl<'tcx> PreDefineMethods<'tcx> for CodegenCx<'_, 'tcx> {
symbol_name: &str,
) {
let instance = Instance::mono(self.tcx, def_id);
let ty = instance.ty(self.tcx, ty::ParamEnv::reveal_all());
let DefKind::Static { nested, .. } = self.tcx.def_kind(def_id) else { bug!() };
// Nested statics do not have a type, so pick a random type and let `define_static` figure out
// the llvm type from the actual evaluated initializer.
let ty = if nested {
self.tcx.types.unit
} else {
instance.ty(self.tcx, ty::ParamEnv::reveal_all())
};
let llty = self.layout_of(ty).llvm_type(self);

let g = self.define_global(symbol_name, llty).unwrap_or_else(|| {
Expand All @@ -38,7 +47,7 @@ impl<'tcx> PreDefineMethods<'tcx> for CodegenCx<'_, 'tcx> {
}
}

self.instances.borrow_mut().insert(instance, g);
self.statics.borrow_mut().insert(def_id, g);
}

fn predefine_fn(
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_codegen_ssa/src/back/symbol_export.rs
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ fn reachable_non_generics_provider(tcx: TyCtxt<'_>, _: LocalCrate) -> DefIdMap<S

// Only consider nodes that actually have exported symbols.
match tcx.def_kind(def_id) {
DefKind::Fn | DefKind::Static(_) => {}
DefKind::Fn | DefKind::Static { .. } => {}
DefKind::AssocFn if tcx.impl_of_method(def_id.to_def_id()).is_some() => {}
_ => return None,
};
Expand Down Expand Up @@ -479,7 +479,7 @@ fn symbol_export_level(tcx: TyCtxt<'_>, sym_def_id: DefId) -> SymbolExportLevel
let target = &tcx.sess.target.llvm_target;
// WebAssembly cannot export data symbols, so reduce their export level
if target.contains("emscripten") {
if let DefKind::Static(_) = tcx.def_kind(sym_def_id) {
if let DefKind::Static { .. } = tcx.def_kind(sym_def_id) {
return SymbolExportLevel::Rust;
}
}
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_codegen_ssa/src/mono_item.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ impl<'a, 'tcx: 'a> MonoItemExt<'a, 'tcx> for MonoItem<'tcx> {

match *self {
MonoItem::Static(def_id) => {
cx.codegen_static(def_id, cx.tcx().is_mutable_static(def_id));
cx.codegen_static(def_id);
}
MonoItem::GlobalAsm(item_id) => {
let item = cx.tcx().hir().item(item_id);
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_codegen_ssa/src/traits/statics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use rustc_target::abi::Align;

pub trait StaticMethods: BackendTypes {
fn static_addr_of(&self, cv: Self::Value, align: Align, kind: Option<&str>) -> Self::Value;
fn codegen_static(&self, def_id: DefId, is_mutable: bool);
fn codegen_static(&self, def_id: DefId);

/// Mark the given global value as "used", to prevent the compiler and linker from potentially
/// removing a static variable that may otherwise appear unused.
Expand Down
10 changes: 5 additions & 5 deletions compiler/rustc_const_eval/src/const_eval/eval_queries.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ fn eval_body_using_ecx<'mir, 'tcx>(
|| matches!(
ecx.tcx.def_kind(cid.instance.def_id()),
DefKind::Const
| DefKind::Static(_)
| DefKind::Static { .. }
| DefKind::ConstParam
| DefKind::AnonConst
| DefKind::InlineConst
Expand All @@ -59,7 +59,7 @@ fn eval_body_using_ecx<'mir, 'tcx>(
};

let ret = if let InternKind::Static(_) = intern_kind {
create_static_alloc(ecx, cid.instance.def_id(), layout)?
create_static_alloc(ecx, cid.instance.def_id().expect_local(), layout)?
} else {
ecx.allocate(layout, MemoryKind::Stack)?
};
Expand Down Expand Up @@ -381,10 +381,10 @@ pub fn eval_in_interpreter<'mir, 'tcx>(
Ok(mplace) => {
// Since evaluation had no errors, validate the resulting constant.

// Temporarily allow access to the static_root_alloc_id for the purpose of validation.
let static_root_alloc_id = ecx.machine.static_root_alloc_id.take();
// Temporarily allow access to the static_root_ids for the purpose of validation.
let static_root_ids = ecx.machine.static_root_ids.take();
let validation = const_validate_mplace(&ecx, &mplace, cid);
ecx.machine.static_root_alloc_id = static_root_alloc_id;
ecx.machine.static_root_ids = static_root_ids;

let alloc_id = mplace.ptr().provenance.unwrap().alloc_id();

Expand Down
Loading

0 comments on commit 51f4acc

Please sign in to comment.