[llvm] de620f5 - [CSPGO] Fix lost IRPGOFlag in CSPGO instrumentation
Rong Xu via llvm-commits
llvm-commits at lists.llvm.org
Tue Aug 24 09:41:36 PDT 2021
Author: Rong Xu
Date: 2021-08-24T09:41:29-07:00
New Revision: de620f5b132b2a84e3e4d08afd42682e36527444
URL: https://github.com/llvm/llvm-project/commit/de620f5b132b2a84e3e4d08afd42682e36527444
DIFF: https://github.com/llvm/llvm-project/commit/de620f5b132b2a84e3e4d08afd42682e36527444.diff
LOG: [CSPGO] Fix lost IRPGOFlag in CSPGO instrumentation
The IRPGOFlag symbol (__llvm_profile_raw_version) is dropped when
identified as non-prevailing for either regular or thin LTO during
the mixed-LTO mode compilation. This happens in the module where
IRPGOFlag is marked as non-prevailing. This variable
is emitted in the final object from the prevailing module.
This is still problematic because we currently query this symbol
to coordinate some actions between PGOInstrumentation pass
and InstrProfiling lowering pass, like whether to do value
profiling, whether to do comdat renaming.
This problem is bought up by YolandaCY in
https://reviews.llvm.org/D107034
YolandCY reported unresolved symbol linker errors in
CSPGO instrumentation build for chromium.
This patch let LTO retain IRPGOFlag decl by adding it to
CompilerUsed list and relax the check in isIRPGOFlagSet() when
doing the InstrProfiling lowering.
The test case in the patch is from D107034
<https://reviews.llvm.org/D107034>.
Differential Revision: https://reviews.llvm.org/D108581
Added:
llvm/test/Transforms/PGOProfile/lto_cspgo_gen.ll
Modified:
llvm/include/llvm/ProfileData/InstrProf.h
llvm/lib/ProfileData/InstrProf.cpp
llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp
llvm/test/Transforms/PGOProfile/Inputs/thinlto_cspgo_bar_gen.ll
llvm/test/Transforms/PGOProfile/thinlto_cspgo_gen.ll
Removed:
################################################################################
diff --git a/llvm/include/llvm/ProfileData/InstrProf.h b/llvm/include/llvm/ProfileData/InstrProf.h
index c0cedb23bdcfd..5100e808e6c50 100644
--- a/llvm/include/llvm/ProfileData/InstrProf.h
+++ b/llvm/include/llvm/ProfileData/InstrProf.h
@@ -1143,8 +1143,8 @@ void getMemOPSizeRangeFromOption(StringRef Str, int64_t &RangeStart,
// Create a COMDAT variable INSTR_PROF_RAW_VERSION_VAR to make the runtime
// aware this is an ir_level profile so it can set the version flag.
-void createIRLevelProfileFlagVar(Module &M, bool IsCS,
- bool InstrEntryBBEnabled);
+GlobalVariable *createIRLevelProfileFlagVar(Module &M, bool IsCS,
+ bool InstrEntryBBEnabled);
// Create the variable for the profile file name.
void createProfileFileNameVar(Module &M, StringRef InstrProfileOutput);
diff --git a/llvm/lib/ProfileData/InstrProf.cpp b/llvm/lib/ProfileData/InstrProf.cpp
index a83b56ed67f15..319f4e774a5d9 100644
--- a/llvm/lib/ProfileData/InstrProf.cpp
+++ b/llvm/lib/ProfileData/InstrProf.cpp
@@ -1098,10 +1098,14 @@ bool needsComdatForCounter(const Function &F, const Module &M) {
bool isIRPGOFlagSet(const Module *M) {
auto IRInstrVar =
M->getNamedGlobal(INSTR_PROF_QUOTE(INSTR_PROF_RAW_VERSION_VAR));
- if (!IRInstrVar || IRInstrVar->isDeclaration() ||
- IRInstrVar->hasLocalLinkage())
+ if (!IRInstrVar || IRInstrVar->hasLocalLinkage())
return false;
+ // For CSPGO+LTO, this variable might be marked as non-prevailing and we only
+ // have the decl.
+ if (IRInstrVar->isDeclaration())
+ return true;
+
// Check if the flag is set.
if (!IRInstrVar->hasInitializer())
return false;
@@ -1137,8 +1141,8 @@ bool canRenameComdatFunc(const Function &F, bool CheckAddressTaken) {
// Create a COMDAT variable INSTR_PROF_RAW_VERSION_VAR to make the runtime
// aware this is an ir_level profile so it can set the version flag.
-void createIRLevelProfileFlagVar(Module &M, bool IsCS,
- bool InstrEntryBBEnabled) {
+GlobalVariable *createIRLevelProfileFlagVar(Module &M, bool IsCS,
+ bool InstrEntryBBEnabled) {
const StringRef VarName(INSTR_PROF_QUOTE(INSTR_PROF_RAW_VERSION_VAR));
Type *IntTy64 = Type::getInt64Ty(M.getContext());
uint64_t ProfileVersion = (INSTR_PROF_RAW_VERSION | VARIANT_MASK_IR_PROF);
@@ -1155,6 +1159,7 @@ void createIRLevelProfileFlagVar(Module &M, bool IsCS,
IRLevelVersionVariable->setLinkage(GlobalValue::ExternalLinkage);
IRLevelVersionVariable->setComdat(M.getOrInsertComdat(VarName));
}
+ return IRLevelVersionVariable;
}
// Create the variable for the profile file name.
diff --git a/llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp b/llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp
index 152fff9ffce05..c437003060cbb 100644
--- a/llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp
+++ b/llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp
@@ -110,6 +110,7 @@
#include "llvm/Support/raw_ostream.h"
#include "llvm/Transforms/Instrumentation.h"
#include "llvm/Transforms/Utils/BasicBlockUtils.h"
+#include "llvm/Transforms/Utils/ModuleUtils.h"
#include <algorithm>
#include <cassert>
#include <cstdint>
@@ -464,7 +465,10 @@ class PGOInstrumentationGenCreateVarLegacyPass : public ModulePass {
private:
bool runOnModule(Module &M) override {
createProfileFileNameVar(M, InstrProfileOutput);
- createIRLevelProfileFlagVar(M, /* IsCS */ true, PGOInstrumentEntry);
+ // The variable in a comdat may be discarded by LTO. Ensure the
+ // declaration will be retained.
+ appendToCompilerUsed(
+ M, createIRLevelProfileFlagVar(M, /*IsCS=*/true, PGOInstrumentEntry));
return false;
}
std::string InstrProfileOutput;
@@ -1612,7 +1616,7 @@ static bool InstrumentAllFunctions(
// For the context-sensitve instrumentation, we should have a separated pass
// (before LTO/ThinLTO linking) to create these variables.
if (!IsCS)
- createIRLevelProfileFlagVar(M, /* IsCS */ false, PGOInstrumentEntry);
+ createIRLevelProfileFlagVar(M, /*IsCS=*/false, PGOInstrumentEntry);
std::unordered_multimap<Comdat *, GlobalValue *> ComdatMembers;
collectComdatMembers(M, ComdatMembers);
@@ -1632,7 +1636,10 @@ static bool InstrumentAllFunctions(
PreservedAnalyses
PGOInstrumentationGenCreateVar::run(Module &M, ModuleAnalysisManager &AM) {
createProfileFileNameVar(M, CSInstrName);
- createIRLevelProfileFlagVar(M, /* IsCS */ true, PGOInstrumentEntry);
+ // The variable in a comdat may be discarded by LTO. Ensure the declaration
+ // will be retained.
+ appendToCompilerUsed(
+ M, createIRLevelProfileFlagVar(M, /*IsCS=*/true, PGOInstrumentEntry));
return PreservedAnalyses::all();
}
diff --git a/llvm/test/Transforms/PGOProfile/Inputs/thinlto_cspgo_bar_gen.ll b/llvm/test/Transforms/PGOProfile/Inputs/thinlto_cspgo_bar_gen.ll
index 42c645db7cd4d..d75bfbc56b735 100644
--- a/llvm/test/Transforms/PGOProfile/Inputs/thinlto_cspgo_bar_gen.ll
+++ b/llvm/test/Transforms/PGOProfile/Inputs/thinlto_cspgo_bar_gen.ll
@@ -1,13 +1,8 @@
target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
target triple = "x86_64-unknown-linux-gnu"
-$__llvm_profile_filename = comdat any
-$__llvm_profile_raw_version = comdat any
-
@odd = common dso_local global i32 0, align 4
@even = common dso_local global i32 0, align 4
- at __llvm_profile_filename = constant [25 x i8] c"pass2/default_%m.profraw\00", comdat
- at __llvm_profile_raw_version = constant i64 216172782113783812, comdat
define dso_local void @bar(i32 %n) !prof !29 {
entry:
diff --git a/llvm/test/Transforms/PGOProfile/lto_cspgo_gen.ll b/llvm/test/Transforms/PGOProfile/lto_cspgo_gen.ll
new file mode 100644
index 0000000000000..8ea2d3ccbfe07
--- /dev/null
+++ b/llvm/test/Transforms/PGOProfile/lto_cspgo_gen.ll
@@ -0,0 +1,32 @@
+; REQUIRES: x86-registered-target
+
+; RUN: opt -passes='thinlto-pre-link<O2>' --cs-profilegen-file=alloc -cspgo-kind=cspgo-instr-gen-pipeline -module-summary %s -o %t.bc
+; RUN: llvm-dis %t.bc -o - | FileCheck %s --check-prefix=IRPGOPRE
+
+;; Symbol __llvm_profile_filename and __llvm_profile_raw_version are non-prevailing here.
+; RUN: llvm-lto2 run -lto-cspgo-profile-file=alloc -lto-cspgo-gen -save-temps -o %t %t.bc \
+; RUN: -r=%t.bc,f,px \
+; RUN: -r=%t.bc,__llvm_profile_filename,x \
+; RUN: -r=%t.bc,__llvm_profile_raw_version,x
+; RUN: llvm-dis %t.0.0.preopt.bc -o - | FileCheck %s --check-prefix=IRPGOBE
+
+;; Before LTO, we should have the __llvm_profile_raw_version definition.
+; IRPGOPRE: @__llvm_profile_raw_version = constant i64
+
+;; Non-prevailing __llvm_profile_raw_version is discarded by LTO. Ensure the
+;; declaration is retained.
+; IRPGOBE: @__llvm_profile_raw_version = external constant i64
+
+target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+$f = comdat any
+
+; Function Attrs: nofree norecurse nosync nounwind readnone uwtable willreturn mustprogress
+define i32 @f() {
+entry:
+ ret i32 1
+}
+
+!llvm.module.flags = !{!0}
+!0 = !{i32 1, !"ThinLTO", i32 0}
diff --git a/llvm/test/Transforms/PGOProfile/thinlto_cspgo_gen.ll b/llvm/test/Transforms/PGOProfile/thinlto_cspgo_gen.ll
index 1a213e8211cce..8a408fa842fb5 100644
--- a/llvm/test/Transforms/PGOProfile/thinlto_cspgo_gen.ll
+++ b/llvm/test/Transforms/PGOProfile/thinlto_cspgo_gen.ll
@@ -1,7 +1,7 @@
; REQUIRES: x86-registered-target
-; RUN: opt -module-summary %s -o %t1.bc
-; RUN: opt -module-summary %S/Inputs/thinlto_cspgo_bar_gen.ll -o %t2.bc
+; RUN: opt -passes='thinlto-pre-link<O2>' --cs-profilegen-file=alloc -cspgo-kind=cspgo-instr-gen-pipeline -module-summary %s -o %t1.bc
+; RUN: opt -passes='thinlto-pre-link<O2>' --cs-profilegen-file=alloc -cspgo-kind=cspgo-instr-gen-pipeline -module-summary %S/Inputs/thinlto_cspgo_bar_gen.ll -o %t2.bc
; RUN: llvm-lto2 run -lto-cspgo-profile-file=alloc -lto-cspgo-gen -save-temps -o %t %t1.bc %t2.bc \
; RUN: -r=%t1.bc,foo,pl \
; RUN: -r=%t1.bc,bar,l \
@@ -13,8 +13,15 @@
; RUN: -r=%t2.bc,even,pl \
; RUN: -r=%t2.bc,__llvm_profile_filename,x \
; RUN: -r=%t2.bc,__llvm_profile_raw_version,x
-; RUN: llvm-dis %t.1.4.opt.bc -o - | FileCheck %s --check-prefix=CSGEN
+; RUN: llvm-dis %t.1.4.opt.bc -o - | FileCheck %s --check-prefixes=CSGEN,PREVAILING
+; RUN: llvm-dis %t.2.4.opt.bc -o - | FileCheck %s --check-prefixes=CSGEN,NOPREVAILING
+;; Prevailing __llvm_profile_raw_version is kept by LTO.
+; PREVAILING: @__llvm_profile_raw_version = constant i64
+
+;; Non-prevailing __llvm_profile_raw_version is discarded by LTO. Ensure the
+;; declaration is retained.
+; NOPREVAILING: @__llvm_profile_raw_version = external constant i64
; CSGEN: @__profc_
; CSGEN: @__profd_
@@ -22,11 +29,6 @@ source_filename = "cspgo.c"
target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
target triple = "x86_64-unknown-linux-gnu"
-$__llvm_profile_filename = comdat any
-$__llvm_profile_raw_version = comdat any
- at __llvm_profile_filename = constant [25 x i8] c"pass2/default_%m.profraw\00", comdat
- at __llvm_profile_raw_version = constant i64 216172782113783812, comdat
-
define dso_local void @foo() #0 !prof !29 {
entry:
br label %for.body
More information about the llvm-commits
mailing list