From 93831cbf8fb957df94ee51cf442a776f27bffc0e Mon Sep 17 00:00:00 2001 From: bjorn3 Date: Thu, 8 Apr 2021 12:08:13 +0200 Subject: [PATCH] Don't deduplicate anonymous allocations --- Cargo.lock | 20 ++++++------ Cargo.toml | 10 +++--- example/mini_core_hello_world.rs | 5 +++ scripts/test_rustc_tests.sh | 1 + src/base.rs | 4 +++ src/common.rs | 2 ++ src/constant.rs | 52 ++++++++++++++++++++------------ src/driver/aot.rs | 2 +- src/driver/jit.rs | 2 +- src/lib.rs | 4 --- 10 files changed, 61 insertions(+), 41 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index dc1cd336e..83947c123 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -40,7 +40,7 @@ checksum = "baf1de4339761588bc0619e3cbc0120ee582ebb74b53b4efbf79117bd2da40fd" [[package]] name = "cranelift-bforest" version = "0.72.0" -source = "git+https://github.com/bytecodealliance/wasmtime/?branch=main#8e43e96410a14143d368273cf1e708f8094bb8e0" +source = "git+https://github.com/bjorn3/wasmtime.git?branch=anon_allocs#cc89111463627afbb9d72939f59307f550406056" dependencies = [ "cranelift-entity", ] @@ -48,7 +48,7 @@ dependencies = [ [[package]] name = "cranelift-codegen" version = "0.72.0" -source = "git+https://github.com/bytecodealliance/wasmtime/?branch=main#8e43e96410a14143d368273cf1e708f8094bb8e0" +source = "git+https://github.com/bjorn3/wasmtime.git?branch=anon_allocs#cc89111463627afbb9d72939f59307f550406056" dependencies = [ "byteorder", "cranelift-bforest", @@ -66,7 +66,7 @@ dependencies = [ [[package]] name = "cranelift-codegen-meta" version = "0.72.0" -source = "git+https://github.com/bytecodealliance/wasmtime/?branch=main#8e43e96410a14143d368273cf1e708f8094bb8e0" +source = "git+https://github.com/bjorn3/wasmtime.git?branch=anon_allocs#cc89111463627afbb9d72939f59307f550406056" dependencies = [ "cranelift-codegen-shared", "cranelift-entity", @@ -75,17 +75,17 @@ dependencies = [ [[package]] name = "cranelift-codegen-shared" version = "0.72.0" -source = "git+https://github.com/bytecodealliance/wasmtime/?branch=main#8e43e96410a14143d368273cf1e708f8094bb8e0" +source = "git+https://github.com/bjorn3/wasmtime.git?branch=anon_allocs#cc89111463627afbb9d72939f59307f550406056" [[package]] name = "cranelift-entity" version = "0.72.0" -source = "git+https://github.com/bytecodealliance/wasmtime/?branch=main#8e43e96410a14143d368273cf1e708f8094bb8e0" +source = "git+https://github.com/bjorn3/wasmtime.git?branch=anon_allocs#cc89111463627afbb9d72939f59307f550406056" [[package]] name = "cranelift-frontend" version = "0.72.0" -source = "git+https://github.com/bytecodealliance/wasmtime/?branch=main#8e43e96410a14143d368273cf1e708f8094bb8e0" +source = "git+https://github.com/bjorn3/wasmtime.git?branch=anon_allocs#cc89111463627afbb9d72939f59307f550406056" dependencies = [ "cranelift-codegen", "log", @@ -96,7 +96,7 @@ dependencies = [ [[package]] name = "cranelift-jit" version = "0.72.0" -source = "git+https://github.com/bytecodealliance/wasmtime/?branch=main#8e43e96410a14143d368273cf1e708f8094bb8e0" +source = "git+https://github.com/bjorn3/wasmtime.git?branch=anon_allocs#cc89111463627afbb9d72939f59307f550406056" dependencies = [ "anyhow", "cranelift-codegen", @@ -114,7 +114,7 @@ dependencies = [ [[package]] name = "cranelift-module" version = "0.72.0" -source = "git+https://github.com/bytecodealliance/wasmtime/?branch=main#8e43e96410a14143d368273cf1e708f8094bb8e0" +source = "git+https://github.com/bjorn3/wasmtime.git?branch=anon_allocs#cc89111463627afbb9d72939f59307f550406056" dependencies = [ "anyhow", "cranelift-codegen", @@ -126,7 +126,7 @@ dependencies = [ [[package]] name = "cranelift-native" version = "0.72.0" -source = "git+https://github.com/bytecodealliance/wasmtime/?branch=main#8e43e96410a14143d368273cf1e708f8094bb8e0" +source = "git+https://github.com/bjorn3/wasmtime.git?branch=anon_allocs#cc89111463627afbb9d72939f59307f550406056" dependencies = [ "cranelift-codegen", "target-lexicon", @@ -135,7 +135,7 @@ dependencies = [ [[package]] name = "cranelift-object" version = "0.72.0" -source = "git+https://github.com/bytecodealliance/wasmtime/?branch=main#8e43e96410a14143d368273cf1e708f8094bb8e0" +source = "git+https://github.com/bjorn3/wasmtime.git?branch=anon_allocs#cc89111463627afbb9d72939f59307f550406056" dependencies = [ "anyhow", "cranelift-codegen", diff --git a/Cargo.toml b/Cargo.toml index 60946ab28..098263f86 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -9,11 +9,11 @@ crate-type = ["dylib"] [dependencies] # These have to be in sync with each other -cranelift-codegen = { git = "https://github.com/bytecodealliance/wasmtime/", branch = "main", features = ["unwind", "x64"] } -cranelift-frontend = { git = "https://github.com/bytecodealliance/wasmtime/", branch = "main" } -cranelift-module = { git = "https://github.com/bytecodealliance/wasmtime/", branch = "main" } -cranelift-jit = { git = "https://github.com/bytecodealliance/wasmtime/", branch = "main", optional = true } -cranelift-object = { git = "https://github.com/bytecodealliance/wasmtime/", branch = "main" } +cranelift-codegen = { git = "https://github.com/bjorn3/wasmtime.git", branch = "anon_allocs", features = ["unwind", "x64"] } +cranelift-frontend = { git = "https://github.com/bjorn3/wasmtime.git", branch = "anon_allocs" } +cranelift-module = { git = "https://github.com/bjorn3/wasmtime.git", branch = "anon_allocs" } +cranelift-jit = { git = "https://github.com/bjorn3/wasmtime.git", branch = "anon_allocs", optional = true } +cranelift-object = { git = "https://github.com/bjorn3/wasmtime.git", branch = "anon_allocs" } target-lexicon = "0.11.0" gimli = { version = "0.23.0", default-features = false, features = ["write"]} object = { version = "0.23.0", default-features = false, features = ["std", "read_core", "write", "archive", "coff", "elf", "macho", "pe"] } diff --git a/example/mini_core_hello_world.rs b/example/mini_core_hello_world.rs index ea37ca98b..47abe2d1d 100644 --- a/example/mini_core_hello_world.rs +++ b/example/mini_core_hello_world.rs @@ -296,6 +296,11 @@ fn main() { unsafe { global_asm_test(); } + + // Both statics have a reference that points to the same anonymous allocation. + static REF1: &u8 = &42; + static REF2: &u8 = REF1; + assert_eq!(*REF1, *REF2); } #[cfg(all(not(jit), target_os = "linux"))] diff --git a/scripts/test_rustc_tests.sh b/scripts/test_rustc_tests.sh index e4b737552..6a76f3a72 100755 --- a/scripts/test_rustc_tests.sh +++ b/scripts/test_rustc_tests.sh @@ -47,6 +47,7 @@ rm src/test/ui/issues/issue-51947.rs # same rm src/test/ui/numbers-arithmetic/saturating-float-casts.rs # intrinsic gives different but valid result rm src/test/ui/mir/mir_misc_casts.rs # depends on deduplication of constants rm src/test/ui/mir/mir_raw_fat_ptr.rs # same +rm src/test/ui/consts/issue-33537.rs # same rm src/test/ui/async-await/async-fn-size-moved-locals.rs # -Cpanic=abort shrinks some generator by one byte rm src/test/ui/async-await/async-fn-size-uninit-locals.rs # same rm src/test/ui/generator/size-moved-locals.rs # same diff --git a/src/base.rs b/src/base.rs index bead6d03c..82f1912c2 100644 --- a/src/base.rs +++ b/src/base.rs @@ -6,6 +6,7 @@ use rustc_middle::ty::adjustment::PointerCast; use rustc_middle::ty::layout::FnAbiExt; use rustc_target::abi::call::FnAbi; +use crate::constant::ConstantCx; use crate::prelude::*; pub(crate) fn codegen_fn<'tcx>(cx: &mut crate::CodegenCx<'_, 'tcx>, instance: Instance<'tcx>) { @@ -47,6 +48,7 @@ pub(crate) fn codegen_fn<'tcx>(cx: &mut crate::CodegenCx<'_, 'tcx>, instance: In tcx, pointer_type, vtables: FxHashMap::default(), + constants_cx: ConstantCx::new(), instance, symbol_name, @@ -92,6 +94,8 @@ pub(crate) fn codegen_fn<'tcx>(cx: &mut crate::CodegenCx<'_, 'tcx>, instance: In let source_info_set = fx.source_info_set; let local_map = fx.local_map; + fx.constants_cx.finalize(fx.tcx, &mut *fx.cx.module); + // Store function in context let context = &mut cx.cached_context; context.func = func; diff --git a/src/common.rs b/src/common.rs index 64618dcf3..6752bb42d 100644 --- a/src/common.rs +++ b/src/common.rs @@ -4,6 +4,7 @@ use rustc_target::abi::call::FnAbi; use rustc_target::abi::{Integer, Primitive}; use rustc_target::spec::{HasTargetSpec, Target}; +use crate::constant::ConstantCx; use crate::prelude::*; pub(crate) fn pointer_ty(tcx: TyCtxt<'_>) -> types::Type { @@ -232,6 +233,7 @@ pub(crate) struct FunctionCx<'m, 'clif, 'tcx> { pub(crate) tcx: TyCtxt<'tcx>, pub(crate) pointer_type: Type, // Cached from module pub(crate) vtables: FxHashMap<(Ty<'tcx>, Option>), DataId>, + pub(crate) constants_cx: ConstantCx, pub(crate) instance: Instance<'tcx>, pub(crate) symbol_name: SymbolName<'tcx>, diff --git a/src/constant.rs b/src/constant.rs index fcd41c844..4f2a60c4b 100644 --- a/src/constant.rs +++ b/src/constant.rs @@ -2,7 +2,7 @@ use rustc_span::DUMMY_SP; -use rustc_data_structures::fx::FxHashSet; +use rustc_data_structures::fx::{FxHashMap, FxHashSet}; use rustc_errors::ErrorReported; use rustc_middle::middle::codegen_fn_attrs::CodegenFnAttrFlags; use rustc_middle::mir::interpret::{ @@ -13,12 +13,12 @@ use rustc_middle::ty::ConstKind; use cranelift_codegen::ir::GlobalValueData; use cranelift_module::*; -use crate::prelude::*; +use crate::{CodegenCx, prelude::*}; -#[derive(Default)] pub(crate) struct ConstantCx { todo: Vec, done: FxHashSet, + anon_allocs: FxHashMap, } #[derive(Copy, Clone, Debug)] @@ -28,6 +28,14 @@ enum TodoItem { } impl ConstantCx { + pub(crate) fn new() -> Self { + ConstantCx { + todo: vec![], + done: FxHashSet::default(), + anon_allocs: FxHashMap::default(), + } + } + pub(crate) fn finalize(mut self, tcx: TyCtxt<'_>, module: &mut dyn Module) { //println!("todo {:?}", self.todo); define_all_allocs(tcx, module, &mut self); @@ -74,8 +82,10 @@ pub(crate) fn check_constants(fx: &mut FunctionCx<'_, '_, '_>) -> bool { all_constants_ok } -pub(crate) fn codegen_static(constants_cx: &mut ConstantCx, def_id: DefId) { +pub(crate) fn codegen_static(cx: &mut CodegenCx<'_, '_>, def_id: DefId) { + let mut constants_cx = ConstantCx::new(); constants_cx.todo.push(TodoItem::Static(def_id)); + constants_cx.finalize(cx.tcx, &mut *cx.module); } pub(crate) fn codegen_tls_ref<'tcx>( @@ -182,9 +192,13 @@ pub(crate) fn codegen_const_value<'tcx>( let alloc_kind = fx.tcx.get_global_alloc(ptr.alloc_id); let base_addr = match alloc_kind { Some(GlobalAlloc::Memory(alloc)) => { - fx.cx.constants_cx.todo.push(TodoItem::Alloc(ptr.alloc_id)); - let data_id = - data_id_for_alloc_id(fx.cx.module, ptr.alloc_id, alloc.mutability); + fx.constants_cx.todo.push(TodoItem::Alloc(ptr.alloc_id)); + let data_id = data_id_for_alloc_id( + &mut fx.constants_cx, + fx.cx.module, + ptr.alloc_id, + alloc.mutability, + ); let local_data_id = fx.cx.module.declare_data_in_func(data_id, &mut fx.bcx.func); if fx.clif_comments.enabled() { @@ -243,8 +257,9 @@ fn pointer_for_allocation<'tcx>( alloc: &'tcx Allocation, ) -> crate::pointer::Pointer { let alloc_id = fx.tcx.create_memory_alloc(alloc); - fx.cx.constants_cx.todo.push(TodoItem::Alloc(alloc_id)); - let data_id = data_id_for_alloc_id(fx.cx.module, alloc_id, alloc.mutability); + fx.constants_cx.todo.push(TodoItem::Alloc(alloc_id)); + let data_id = + data_id_for_alloc_id(&mut fx.constants_cx, &mut *fx.cx.module, alloc_id, alloc.mutability); let local_data_id = fx.cx.module.declare_data_in_func(data_id, &mut fx.bcx.func); if fx.clif_comments.enabled() { @@ -255,18 +270,16 @@ fn pointer_for_allocation<'tcx>( } fn data_id_for_alloc_id( + cx: &mut ConstantCx, module: &mut dyn Module, alloc_id: AllocId, mutability: rustc_hir::Mutability, ) -> DataId { - module - .declare_data( - &format!(".L__alloc_{:x}", alloc_id.0), - Linkage::Local, - mutability == rustc_hir::Mutability::Mut, - false, - ) + *cx.anon_allocs.entry(alloc_id).or_insert_with(|| { + module + .declare_anonymous_data(mutability == rustc_hir::Mutability::Mut, false) .unwrap() + }) } fn data_id_for_static( @@ -344,7 +357,7 @@ fn define_all_allocs(tcx: TyCtxt<'_>, module: &mut dyn Module, cx: &mut Constant GlobalAlloc::Memory(alloc) => alloc, GlobalAlloc::Function(_) | GlobalAlloc::Static(_) => unreachable!(), }; - let data_id = data_id_for_alloc_id(module, alloc_id, alloc.mutability); + let data_id = data_id_for_alloc_id(cx, module, alloc_id, alloc.mutability); (data_id, alloc, None) } TodoItem::Static(def_id) => { @@ -397,7 +410,7 @@ fn define_all_allocs(tcx: TyCtxt<'_>, module: &mut dyn Module, cx: &mut Constant } GlobalAlloc::Memory(target_alloc) => { cx.todo.push(TodoItem::Alloc(reloc)); - data_id_for_alloc_id(module, reloc, target_alloc.mutability) + data_id_for_alloc_id(cx, module, reloc, target_alloc.mutability) } GlobalAlloc::Static(def_id) => { if tcx.codegen_fn_attrs(def_id).flags.contains(CodegenFnAttrFlags::THREAD_LOCAL) @@ -419,8 +432,7 @@ fn define_all_allocs(tcx: TyCtxt<'_>, module: &mut dyn Module, cx: &mut Constant data_ctx.write_data_addr(offset.bytes() as u32, global_value, addend as i64); } - // FIXME don't duplicate definitions in lazy jit mode - let _ = module.define_data(data_id, &data_ctx); + module.define_data(data_id, &data_ctx).unwrap(); cx.done.insert(data_id); } diff --git a/src/driver/aot.rs b/src/driver/aot.rs index 6ce174eec..3f9f49e88 100644 --- a/src/driver/aot.rs +++ b/src/driver/aot.rs @@ -122,7 +122,7 @@ fn module_codegen( cx.tcx.sess.time("codegen fn", || crate::base::codegen_fn(&mut cx, inst)); } MonoItem::Static(def_id) => { - crate::constant::codegen_static(&mut cx.constants_cx, def_id) + crate::constant::codegen_static(&mut cx, def_id) } MonoItem::GlobalAsm(item_id) => { let item = cx.tcx.hir().item(item_id); diff --git a/src/driver/jit.rs b/src/driver/jit.rs index e19283af1..ffe449115 100644 --- a/src/driver/jit.rs +++ b/src/driver/jit.rs @@ -58,7 +58,7 @@ pub(super) fn run_jit(tcx: TyCtxt<'_>, backend_config: BackendConfig) -> ! { CodegenMode::JitLazy => codegen_shim(&mut cx, inst), }, MonoItem::Static(def_id) => { - crate::constant::codegen_static(&mut cx.constants_cx, def_id); + crate::constant::codegen_static(&mut cx, def_id); } MonoItem::GlobalAsm(item_id) => { let item = cx.tcx.hir().item(item_id); diff --git a/src/lib.rs b/src/lib.rs index 9aec0bc82..e2d3c7554 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -36,7 +36,6 @@ use rustc_session::Session; use cranelift_codegen::settings::{self, Configurable}; pub use crate::config::*; -use crate::constant::ConstantCx; use crate::prelude::*; mod abi; @@ -123,7 +122,6 @@ struct CodegenCx<'m, 'tcx: 'm> { tcx: TyCtxt<'tcx>, module: &'m mut dyn Module, global_asm: String, - constants_cx: ConstantCx, cached_context: Context, debug_context: Option>, unwind_context: UnwindContext<'tcx>, @@ -147,7 +145,6 @@ impl<'m, 'tcx> CodegenCx<'m, 'tcx> { tcx, module, global_asm: String::new(), - constants_cx: ConstantCx::default(), cached_context: Context::new(), debug_context, unwind_context, @@ -155,7 +152,6 @@ impl<'m, 'tcx> CodegenCx<'m, 'tcx> { } fn finalize(self) -> (String, Option>, UnwindContext<'tcx>) { - self.constants_cx.finalize(self.tcx, self.module); (self.global_asm, self.debug_context, self.unwind_context) } }