From 4a76934aa7e46acad5f150ff394601c0b6d6d53f Mon Sep 17 00:00:00 2001 From: Boris-Chengbiao Zhou Date: Tue, 25 May 2021 23:54:07 +0200 Subject: [PATCH] Fix static relocation model for PowerPC64 We now also use `should_assume_dso_local()` for declarations and port two additional cases from clang: - Exclude PPC64 [1] - Exclude thread-local variables [2] [1]: https://github.com/llvm/llvm-project/blob/033138ea452f5f493fb5095e5963419905ad12e1/clang/lib/CodeGen/CodeGenModule.cpp#L1038-L1040 [2]: https://github.com/llvm/llvm-project/blob/033138ea452f5f493fb5095e5963419905ad12e1/clang/lib/CodeGen/CodeGenModule.cpp#L1048-L1050 --- compiler/rustc_codegen_llvm/src/base.rs | 24 ----------- compiler/rustc_codegen_llvm/src/callee.rs | 3 +- compiler/rustc_codegen_llvm/src/consts.rs | 9 ++--- compiler/rustc_codegen_llvm/src/llvm/ffi.rs | 3 +- compiler/rustc_codegen_llvm/src/mono_item.rs | 42 +++++++++++++++----- src/test/assembly/static-relocation-model.rs | 11 ++++- 6 files changed, 46 insertions(+), 46 deletions(-) diff --git a/compiler/rustc_codegen_llvm/src/base.rs b/compiler/rustc_codegen_llvm/src/base.rs index b296db64ee9bd..893c909b20416 100644 --- a/compiler/rustc_codegen_llvm/src/base.rs +++ b/compiler/rustc_codegen_llvm/src/base.rs @@ -218,27 +218,3 @@ pub fn visibility_to_llvm(linkage: Visibility) -> llvm::Visibility { Visibility::Protected => llvm::Visibility::Protected, } } - -pub fn linkage_from_llvm(linkage: llvm::Linkage) -> Linkage { - match linkage { - llvm::Linkage::ExternalLinkage => Linkage::External, - llvm::Linkage::AvailableExternallyLinkage => Linkage::AvailableExternally, - llvm::Linkage::LinkOnceAnyLinkage => Linkage::LinkOnceAny, - llvm::Linkage::LinkOnceODRLinkage => Linkage::LinkOnceODR, - llvm::Linkage::WeakAnyLinkage => Linkage::WeakAny, - llvm::Linkage::WeakODRLinkage => Linkage::WeakODR, - llvm::Linkage::AppendingLinkage => Linkage::Appending, - llvm::Linkage::InternalLinkage => Linkage::Internal, - llvm::Linkage::PrivateLinkage => Linkage::Private, - llvm::Linkage::ExternalWeakLinkage => Linkage::ExternalWeak, - llvm::Linkage::CommonLinkage => Linkage::Common, - } -} - -pub fn visibility_from_llvm(linkage: llvm::Visibility) -> Visibility { - match linkage { - llvm::Visibility::Default => Visibility::Default, - llvm::Visibility::Hidden => Visibility::Hidden, - llvm::Visibility::Protected => Visibility::Protected, - } -} diff --git a/compiler/rustc_codegen_llvm/src/callee.rs b/compiler/rustc_codegen_llvm/src/callee.rs index b26969a50120f..bb16c90cd12f1 100644 --- a/compiler/rustc_codegen_llvm/src/callee.rs +++ b/compiler/rustc_codegen_llvm/src/callee.rs @@ -14,7 +14,6 @@ use tracing::debug; use rustc_middle::ty::layout::{FnAbiExt, HasTyCtxt}; use rustc_middle::ty::{self, Instance, TypeFoldable}; -use rustc_target::spec::RelocModel; /// Codegens a reference to a fn/method item, monomorphizing and /// inlining as it goes. @@ -181,7 +180,7 @@ pub fn get_fn(cx: &CodegenCx<'ll, 'tcx>, instance: Instance<'tcx>) -> &'ll Value llvm::LLVMSetDLLStorageClass(llfn, llvm::DLLStorageClass::DllImport); } - if cx.tcx.sess.relocation_model() == RelocModel::Static { + if cx.should_assume_dso_local(llfn, true) { llvm::LLVMRustSetDSOLocal(llfn, true); } } diff --git a/compiler/rustc_codegen_llvm/src/consts.rs b/compiler/rustc_codegen_llvm/src/consts.rs index 245842df1b060..e50d5506e222f 100644 --- a/compiler/rustc_codegen_llvm/src/consts.rs +++ b/compiler/rustc_codegen_llvm/src/consts.rs @@ -17,7 +17,6 @@ use rustc_middle::mir::mono::MonoItem; use rustc_middle::ty::{self, Instance, Ty}; use rustc_middle::{bug, span_bug}; use rustc_target::abi::{AddressSpace, Align, HasDataLayout, LayoutOf, Primitive, Scalar, Size}; -use rustc_target::spec::RelocModel; use tracing::debug; pub fn const_alloc_to_llvm(cx: &CodegenCx<'ll, '_>, alloc: &Allocation) -> &'ll Value { @@ -283,8 +282,8 @@ impl CodegenCx<'ll, 'tcx> { } } - if self.tcx.sess.relocation_model() == RelocModel::Static { - unsafe { + unsafe { + if self.should_assume_dso_local(g, true) { llvm::LLVMRustSetDSOLocal(g, true); } } @@ -370,9 +369,7 @@ impl StaticMethods for CodegenCx<'ll, 'tcx> { set_global_alignment(&self, g, self.align_of(ty)); llvm::LLVMSetInitializer(g, v); - let linkage = base::linkage_from_llvm(llvm::LLVMRustGetLinkage(g)); - let visibility = base::visibility_from_llvm(llvm::LLVMRustGetVisibility(g)); - if self.should_assume_dso_local(linkage, visibility) { + if self.should_assume_dso_local(g, true) { llvm::LLVMRustSetDSOLocal(g, true); } diff --git a/compiler/rustc_codegen_llvm/src/llvm/ffi.rs b/compiler/rustc_codegen_llvm/src/llvm/ffi.rs index 966be4a53fd5a..8b1dcea3fa262 100644 --- a/compiler/rustc_codegen_llvm/src/llvm/ffi.rs +++ b/compiler/rustc_codegen_llvm/src/llvm/ffi.rs @@ -72,7 +72,7 @@ pub enum Linkage { // LLVMRustVisibility #[repr(C)] -#[derive(Copy, Clone)] +#[derive(Copy, Clone, PartialEq)] pub enum Visibility { Default = 0, Hidden = 1, @@ -1035,6 +1035,7 @@ extern "C" { pub fn LLVMDeleteGlobal(GlobalVar: &Value); pub fn LLVMGetInitializer(GlobalVar: &Value) -> Option<&Value>; pub fn LLVMSetInitializer(GlobalVar: &'a Value, ConstantVal: &'a Value); + pub fn LLVMIsThreadLocal(GlobalVar: &Value) -> Bool; pub fn LLVMSetThreadLocal(GlobalVar: &Value, IsThreadLocal: Bool); pub fn LLVMSetThreadLocalMode(GlobalVar: &Value, Mode: ThreadLocalMode); pub fn LLVMIsGlobalConstant(GlobalVar: &Value) -> Bool; diff --git a/compiler/rustc_codegen_llvm/src/mono_item.rs b/compiler/rustc_codegen_llvm/src/mono_item.rs index fc1f364e9c6bc..93456443aa015 100644 --- a/compiler/rustc_codegen_llvm/src/mono_item.rs +++ b/compiler/rustc_codegen_llvm/src/mono_item.rs @@ -37,7 +37,7 @@ impl PreDefineMethods<'tcx> for CodegenCx<'ll, 'tcx> { unsafe { llvm::LLVMRustSetLinkage(g, base::linkage_to_llvm(linkage)); llvm::LLVMRustSetVisibility(g, base::visibility_to_llvm(visibility)); - if self.should_assume_dso_local(linkage, visibility) { + if self.should_assume_dso_local(g, false) { llvm::LLVMRustSetDSOLocal(g, true); } } @@ -85,7 +85,7 @@ impl PreDefineMethods<'tcx> for CodegenCx<'ll, 'tcx> { attributes::from_fn_attrs(self, lldecl, instance); unsafe { - if self.should_assume_dso_local(linkage, visibility) { + if self.should_assume_dso_local(lldecl, false) { llvm::LLVMRustSetDSOLocal(lldecl, true); } } @@ -95,28 +95,48 @@ impl PreDefineMethods<'tcx> for CodegenCx<'ll, 'tcx> { } impl CodegenCx<'ll, 'tcx> { - /// Whether a definition (NB: not declaration!) can be assumed to be local to a group of + /// Whether a definition or declaration can be assumed to be local to a group of /// libraries that form a single DSO or executable. pub(crate) unsafe fn should_assume_dso_local( &self, - linkage: Linkage, - visibility: Visibility, + llval: &llvm::Value, + is_declaration: bool, ) -> bool { - if matches!(linkage, Linkage::Internal | Linkage::Private) { + let linkage = llvm::LLVMRustGetLinkage(llval); + let visibility = llvm::LLVMRustGetVisibility(llval); + + if matches!(linkage, llvm::Linkage::InternalLinkage | llvm::Linkage::PrivateLinkage) { return true; } - if visibility != Visibility::Default && linkage != Linkage::ExternalWeak { + if visibility != llvm::Visibility::Default && linkage != llvm::Linkage::ExternalWeakLinkage + { return true; } - // Static relocation model should force copy relocations everywhere. - if self.tcx.sess.relocation_model() == RelocModel::Static { + // Symbols from executables can't really be imported any further. + let all_exe = self.tcx.sess.crate_types().iter().all(|ty| *ty == CrateType::Executable); + let is_declaration_for_linker = + is_declaration || linkage == llvm::Linkage::AvailableExternallyLinkage; + if all_exe && !is_declaration_for_linker { return true; } - // Symbols from executables can't really be imported any further. - if self.tcx.sess.crate_types().iter().all(|ty| *ty == CrateType::Executable) { + // PowerPC64 prefers TOC indirection to avoid copy relocations. + if matches!(&*self.tcx.sess.target.arch, "powerpc64" | "powerpc64le") { + return false; + } + + // Thread-local variables generally don't support copy relocations. + let is_thread_local_var = llvm::LLVMIsAGlobalVariable(llval) + .map(|v| llvm::LLVMIsThreadLocal(v) == llvm::True) + .unwrap_or(false); + if is_thread_local_var { + return false; + } + + // Static relocation model should force copy relocations everywhere. + if self.tcx.sess.relocation_model() == RelocModel::Static { return true; } diff --git a/src/test/assembly/static-relocation-model.rs b/src/test/assembly/static-relocation-model.rs index ce2b3b1cfa414..2cd74a01c8424 100644 --- a/src/test/assembly/static-relocation-model.rs +++ b/src/test/assembly/static-relocation-model.rs @@ -1,9 +1,10 @@ // min-llvm-version: 12.0.0 -// needs-llvm-components: aarch64 x86 -// revisions:x64 A64 +// needs-llvm-components: aarch64 x86 powerpc +// revisions: x64 A64 ppc64le // assembly-output: emit-asm // [x64] compile-flags: --target x86_64-unknown-linux-gnu -Crelocation-model=static // [A64] compile-flags: --target aarch64-unknown-linux-gnu -Crelocation-model=static +// [ppc64le] compile-flags: --target powerpc64le-unknown-linux-gnu -Crelocation-model=static #![feature(no_core, lang_items)] #![no_core] @@ -75,3 +76,9 @@ pub fn mango() -> u8 { pub fn orange() -> &'static u8 { &PIERIS } + +// For ppc64 we need to make sure to generate TOC entries even with the static relocation model +// ppc64le: .tc chaenomeles[TC],chaenomeles +// ppc64le: .tc banana[TC],banana +// ppc64le: .tc EXOCHORDA[TC],EXOCHORDA +// ppc64le: .tc PIERIS[TC],PIERIS