[compiler-rt] 33a7b4d - [InstrProfiling] Use external weak reference for bias variable
Petr Hosek via llvm-commits
llvm-commits at lists.llvm.org
Thu Jul 1 15:25:56 PDT 2021
Author: Petr Hosek
Date: 2021-07-01T15:25:31-07:00
New Revision: 33a7b4d9d8e6a113108aa71ed78ca32a83c68523
URL: https://github.com/llvm/llvm-project/commit/33a7b4d9d8e6a113108aa71ed78ca32a83c68523
DIFF: https://github.com/llvm/llvm-project/commit/33a7b4d9d8e6a113108aa71ed78ca32a83c68523.diff
LOG: [InstrProfiling] Use external weak reference for bias variable
We need the compiler generated variable to override the weak symbol of
the same name inside the profile runtime, but using LinkOnceODRLinkage
results in weak symbol being emitted which leads to an issue where the
linker might choose either of the weak symbols potentially disabling the
runtime counter relocation.
This change replaces the use of weak definition inside the runtime with
an external weak reference to address the issue. We also place the
compiler generated symbol inside a COMDAT group so dead definition can
be garbage collected by the linker.
Differential Revision: https://reviews.llvm.org/D105176
Added:
Modified:
compiler-rt/lib/profile/CMakeLists.txt
compiler-rt/lib/profile/InstrProfiling.h
compiler-rt/lib/profile/InstrProfilingFile.c
compiler-rt/lib/profile/InstrProfilingPlatformFuchsia.c
llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp
llvm/test/Instrumentation/InstrProfiling/runtime-counter-relocation.ll
Removed:
compiler-rt/lib/profile/InstrProfilingBiasVar.c
################################################################################
diff --git a/compiler-rt/lib/profile/CMakeLists.txt b/compiler-rt/lib/profile/CMakeLists.txt
index f899e402d9222..f5e13574b7ce8 100644
--- a/compiler-rt/lib/profile/CMakeLists.txt
+++ b/compiler-rt/lib/profile/CMakeLists.txt
@@ -53,7 +53,6 @@ set(PROFILE_SOURCES
InstrProfiling.c
InstrProfilingInternal.c
InstrProfilingValue.c
- InstrProfilingBiasVar.c
InstrProfilingBuffer.c
InstrProfilingFile.c
InstrProfilingMerge.c
diff --git a/compiler-rt/lib/profile/InstrProfiling.h b/compiler-rt/lib/profile/InstrProfiling.h
index 39fe4db73da62..1c0e8f3c5c8ca 100644
--- a/compiler-rt/lib/profile/InstrProfiling.h
+++ b/compiler-rt/lib/profile/InstrProfiling.h
@@ -320,10 +320,10 @@ extern uint64_t INSTR_PROF_RAW_VERSION_VAR; /* __llvm_profile_raw_version */
extern char INSTR_PROF_PROFILE_NAME_VAR[1]; /* __llvm_profile_filename. */
/*!
- * This variable is a weak symbol defined in InstrProfilingBiasVar.c. It
- * allows compiler instrumentation to provide overriding definition with
- * value from compiler command line. This variable has hidden visibility.
+ * This variable is a weak external reference which could be used to detect
+ * whether or not the compiler defined this symbol.
*/
-COMPILER_RT_VISIBILITY extern intptr_t __llvm_profile_counter_bias;
+COMPILER_RT_VISIBILITY COMPILER_RT_WEAK extern intptr_t
+ __llvm_profile_counter_bias;
#endif /* PROFILE_INSTRPROFILING_H_ */
diff --git a/compiler-rt/lib/profile/InstrProfilingBiasVar.c b/compiler-rt/lib/profile/InstrProfilingBiasVar.c
deleted file mode 100644
index 05745fd858d97..0000000000000
--- a/compiler-rt/lib/profile/InstrProfilingBiasVar.c
+++ /dev/null
@@ -1,15 +0,0 @@
-/*===- InstrProfilingBiasVar.c - profile counter bias variable setup ------===*\
-|*
-|* Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
-|* See https://llvm.org/LICENSE.txt for license information.
-|* SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-|*
-\*===----------------------------------------------------------------------===*/
-
-#include "InstrProfiling.h"
-
-/* The runtime should only provide its own definition of this symbol when the
- * user has not specified one. Set this up by moving the runtime's copy of this
- * symbol to an object file within the archive.
- */
-COMPILER_RT_WEAK intptr_t __llvm_profile_counter_bias = -1;
diff --git a/compiler-rt/lib/profile/InstrProfilingFile.c b/compiler-rt/lib/profile/InstrProfilingFile.c
index 420e8246f4337..d88531cbcb633 100644
--- a/compiler-rt/lib/profile/InstrProfilingFile.c
+++ b/compiler-rt/lib/profile/InstrProfilingFile.c
@@ -999,7 +999,9 @@ void __llvm_profile_initialize_file(void) {
ProfileNameSpecifier PNS = PNS_unknown;
int hasCommandLineOverrider = (INSTR_PROF_PROFILE_NAME_VAR[0] != 0);
- if (__llvm_profile_counter_bias != -1)
+ /* This symbol is defined by the compiler when runtime counter relocation is
+ * used and runtime provides a weak external reference so we can check it. */
+ if (&__llvm_profile_counter_bias)
lprofSetRuntimeCounterRelocation(1);
EnvFilenamePat = getFilenamePatFromEnv();
diff --git a/compiler-rt/lib/profile/InstrProfilingPlatformFuchsia.c b/compiler-rt/lib/profile/InstrProfilingPlatformFuchsia.c
index 8bd5e969aa50c..31f3e11a072e1 100644
--- a/compiler-rt/lib/profile/InstrProfilingPlatformFuchsia.c
+++ b/compiler-rt/lib/profile/InstrProfilingPlatformFuchsia.c
@@ -116,10 +116,9 @@ void __llvm_profile_initialize(void) {
return;
}
- /* This symbol is defined as weak and initialized to -1 by the runtimer, but
- * compiler will generate a strong definition initialized to 0 when runtime
- * counter relocation is used. */
- if (__llvm_profile_counter_bias == -1) {
+ /* This symbol is defined by the compiler when runtime counter relocation is
+ * used and runtime provides a weak external reference so we can check it. */
+ if (!&__llvm_profile_counter_bias) {
lprofWrite("LLVM Profile: counter relocation at runtime is required\n");
return;
}
diff --git a/llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp b/llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp
index 721f8c034438f..9264f83156c55 100644
--- a/llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp
+++ b/llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp
@@ -690,10 +690,19 @@ void InstrProfiling::lowerIncrement(InstrProfIncrementInst *Inc) {
Type *Int64Ty = Type::getInt64Ty(M->getContext());
GlobalVariable *Bias = M->getGlobalVariable(getInstrProfCounterBiasVarName());
if (!Bias) {
+ // Compiler must define this variable when runtime counter relocation
+ // is being used. Runtime has a weak external reference that is used
+ // to check whether that's the case or not.
Bias = new GlobalVariable(*M, Int64Ty, false, GlobalValue::LinkOnceODRLinkage,
Constant::getNullValue(Int64Ty),
getInstrProfCounterBiasVarName());
Bias->setVisibility(GlobalVariable::HiddenVisibility);
+ // A definition that's weak (linkonce_odr) without being in a COMDAT
+ // section wouldn't lead to link errors, but it would lead to a dead
+ // data word from every TU but one. Putting it in COMDAT ensures there
+ // will be exactly one data slot in the link.
+ if (TT.supportsCOMDAT())
+ Bias->setComdat(M->getOrInsertComdat(Bias->getName()));
}
LI = Builder.CreateLoad(Int64Ty, Bias);
}
diff --git a/llvm/test/Instrumentation/InstrProfiling/runtime-counter-relocation.ll b/llvm/test/Instrumentation/InstrProfiling/runtime-counter-relocation.ll
index 672492474c5ff..cd5d36b8a6e3e 100644
--- a/llvm/test/Instrumentation/InstrProfiling/runtime-counter-relocation.ll
+++ b/llvm/test/Instrumentation/InstrProfiling/runtime-counter-relocation.ll
@@ -4,7 +4,8 @@
target triple = "x86_64-unknown-linux-gnu"
@__profn_foo = private constant [3 x i8] c"foo"
-; RELOC: @__llvm_profile_counter_bias = linkonce_odr hidden global i64 0
+; RELOC: $__llvm_profile_counter_bias = comdat any
+; RELOC: @__llvm_profile_counter_bias = linkonce_odr hidden global i64 0, comdat
; CHECK-LABEL: define void @foo
; CHECK-NEXT: %pgocount = load i64, i64* getelementptr inbounds ([1 x i64], [1 x i64]* @__profc_foo, i64 0, i64 0)
More information about the llvm-commits
mailing list