Index: include/llvm/ProfileData/InstrProf.h =================================================================== --- include/llvm/ProfileData/InstrProf.h +++ include/llvm/ProfileData/InstrProf.h @@ -230,6 +230,13 @@ /// bytes. This method decodes the string and populates the \c Symtab. Error readPGOFuncNameStrings(StringRef NameStrings, InstrProfSymtab &Symtab); +/// Check if INSTR_PROF_RAW_VERSION_VAR is defined. This global is only being +/// set in IR PGO compilation. +bool isIRPGOFlagSet(const Module *M); + +/// Check if we can safely rename this Comdat function. +bool canRenameComdatInIRPGO(const Function &F); + enum InstrProfValueKind : uint32_t { #define VALUE_PROF_KIND(Enumerator, Value) Enumerator = Value, #include "llvm/ProfileData/InstrProfData.inc" Index: lib/ProfileData/InstrProf.cpp =================================================================== --- lib/ProfileData/InstrProf.cpp +++ lib/ProfileData/InstrProf.cpp @@ -811,4 +811,43 @@ return true; } + +// Check if INSTR_PROF_RAW_VERSION_VAR is defined. +bool isIRPGOFlagSet(const Module *M) { + auto IRInstrVar = + M->getNamedGlobal(INSTR_PROF_QUOTE(INSTR_PROF_RAW_VERSION_VAR)); + if (!IRInstrVar || IRInstrVar->isDeclaration() || + IRInstrVar->hasLocalLinkage()) + return false; + + // Check if the flag is set. + if (!IRInstrVar->hasInitializer()) + return false; + + const Constant *InitVal = IRInstrVar->getInitializer(); + if (!InitVal) + return false; + + return (dyn_cast(InitVal)->getZExtValue() & + VARIANT_MASK_IR_PROF) != 0; +} + +// Check if we can safely rename this Comdat function. +bool canRenameComdatInIRPGO(const Function &F) { + if (F.getName().empty()) + return false; + if (!needsComdatForCounter(F, *(F.getParent()))) + return false; + // Only safe to do if this function may be discarded if it is not used + // in the compilation unit. + if (!GlobalValue::isDiscardableIfUnused(F.getLinkage())) + return false; + + // For AvailableExternallyLinkage functions. + if (!F.hasComdat()) { + assert(F.getLinkage() == GlobalValue::AvailableExternallyLinkage); + return true; + } + return true; +} } // end namespace llvm Index: lib/Transforms/Instrumentation/InstrProfiling.cpp =================================================================== --- lib/Transforms/Instrumentation/InstrProfiling.cpp +++ lib/Transforms/Instrumentation/InstrProfiling.cpp @@ -32,6 +32,10 @@ cl::desc("Enable name string compression"), cl::init(true)); +cl::opt DoInstrprofVarHashpostfix( + "instrprof-var-hashpostfix", + cl::desc("Append hash to the instrprof variables"), cl::init(true)); + cl::opt ValueProfileStaticAlloc( "vp-static-alloc", cl::desc("Do static counter allocation for value profiler"), @@ -272,7 +276,19 @@ static std::string getVarName(InstrProfIncrementInst *Inc, StringRef Prefix) { StringRef NamePrefix = getInstrProfNameVarPrefix(); StringRef Name = Inc->getName()->getName().substr(NamePrefix.size()); - return (Prefix + Name).str(); + Function *F = Inc->getParent()->getParent(); + Module *M = F->getParent(); + if (!DoInstrprofVarHashpostfix || !isIRPGOFlagSet(M) || + !canRenameComdatInIRPGO(*F)) + return (Prefix + Name).str(); + uint64_t FuncHash = Inc->getHash()->getZExtValue(); + // No hash: don't appent it to the profile variable. + if (FuncHash == 0) + return (Prefix + Name).str(); + SmallVector HashPostfix; + if (Name.endswith((Twine(".") + Twine(FuncHash)).toStringRef(HashPostfix))) + return (Prefix + Name).str(); + return (Prefix + Name + "." + Twine(FuncHash)).str(); } static inline bool shouldRecordFunctionAddr(Function *F) { Index: lib/Transforms/Instrumentation/PGOInstrumentation.cpp =================================================================== --- lib/Transforms/Instrumentation/PGOInstrumentation.cpp +++ lib/Transforms/Instrumentation/PGOInstrumentation.cpp @@ -119,7 +119,7 @@ // Command line option to control appending FunctionHash to the name of a COMDAT // function. This is to avoid the hash mismatch caused by the preinliner. static cl::opt DoComdatRenaming( - "do-comdat-renaming", cl::init(true), cl::Hidden, + "do-comdat-renaming", cl::init(false), cl::Hidden, cl::desc("Append function hash to the name of COMDAT function to avoid " "function hash mismatch due to the preinliner")); @@ -407,20 +407,8 @@ static bool canRenameComdat( Function &F, std::unordered_multimap &ComdatMembers) { - if (F.getName().empty()) + if (!canRenameComdatInIRPGO(F)) return false; - if (!needsComdatForCounter(F, *(F.getParent()))) - return false; - // Only safe to do if this function may be discarded if it is not used - // in the compilation unit. - if (!GlobalValue::isDiscardableIfUnused(F.getLinkage())) - return false; - - // For AvailableExternallyLinkage functions. - if (!F.hasComdat()) { - assert(F.getLinkage() == GlobalValue::AvailableExternallyLinkage); - return true; - } // FIXME: Current only handle those Comdat groups that only containing one // function and function aliases. Index: test/Transforms/PGOProfile/comdat_internal.ll =================================================================== --- test/Transforms/PGOProfile/comdat_internal.ll +++ test/Transforms/PGOProfile/comdat_internal.ll @@ -4,17 +4,17 @@ target triple = "x86_64-unknown-linux-gnu" $foo = comdat any -; CHECK: $foo.[[FOO_HASH:[0-9]+]] = comdat any +; CHECK: $foo = comdat any ; CHECK: $__llvm_profile_raw_version = comdat any -; CHECK: $__profv__stdin__foo.[[FOO_HASH]] = comdat any +; CHECK: $__profv__stdin__foo.[[FOO_HASH:[0-9]+]] = comdat any @bar = global i32 ()* @foo, align 8 ; CHECK: @__llvm_profile_raw_version = constant i64 {{[0-9]+}}, comdat -; CHECK: @__profn__stdin__foo.[[FOO_HASH]] = private constant [23 x i8] c":foo.[[FOO_HASH]]" +; CHECK: @__profn__stdin__foo = private constant [11 x i8] c":foo" ; CHECK: @__profc__stdin__foo.[[FOO_HASH]] = private global [1 x i64] zeroinitializer, section "__llvm_prf_cnts", comdat($__profv__stdin__foo.[[FOO_HASH]]), align 8 -; CHECK: @__profd__stdin__foo.[[FOO_HASH]] = private global { i64, i64, i64*, i8*, i8*, i32, [1 x i16] } { i64 6965568665848889497, i64 [[FOO_HASH]], i64* getelementptr inbounds ([1 x i64], [1 x i64]* @__profc__stdin__foo.[[FOO_HASH]], i32 0, i32 0), i8* null +; CHECK: @__profd__stdin__foo.[[FOO_HASH]] = private global { i64, i64, i64*, i8*, i8*, i32, [1 x i16] } { i64 -5640069336071256030, i64 [[FOO_HASH]], i64* getelementptr inbounds ([1 x i64], [1 x i64]* @__profc__stdin__foo.[[FOO_HASH]], i32 0, i32 0), i8* null ; CHECK-NOT: bitcast (i32 ()* @foo to i8*) ; CHECK-SAME: , i8* null, i32 1, [1 x i16] zeroinitializer }, section "__llvm_prf_data", comdat($__profv__stdin__foo.[[FOO_HASH]]), align 8 ; CHECK: @__llvm_prf_nm Index: test/Transforms/PGOProfile/comdat_rename.ll =================================================================== --- test/Transforms/PGOProfile/comdat_rename.ll +++ test/Transforms/PGOProfile/comdat_rename.ll @@ -1,7 +1,7 @@ -; RUN: opt < %s -mtriple=x86_64-unknown-linux -pgo-instr-gen -S | FileCheck --check-prefixes COMMON,ELFONLY %s -; RUN: opt < %s -mtriple=x86_64-unknown-linux -passes=pgo-instr-gen -S | FileCheck --check-prefixes COMMON,ELFONLY %s -; RUN: opt < %s -mtriple=x86_64-pc-win32-coff -pgo-instr-gen -S | FileCheck --check-prefixes COMMON,COFFONLY %s -; RUN: opt < %s -mtriple=x86_64-pc-win32-coff -passes=pgo-instr-gen -S | FileCheck --check-prefixes COMMON,COFFONLY %s +; RUN: opt < %s -mtriple=x86_64-unknown-linux -pgo-instr-gen -do-comdat-renaming=true -S | FileCheck --check-prefixes COMMON,ELFONLY %s +; RUN: opt < %s -mtriple=x86_64-unknown-linux -passes=pgo-instr-gen -do-comdat-renaming=true -S | FileCheck --check-prefixes COMMON,ELFONLY %s +; RUN: opt < %s -mtriple=x86_64-pc-win32-coff -pgo-instr-gen -do-comdat-renaming=true -S | FileCheck --check-prefixes COMMON,COFFONLY %s +; RUN: opt < %s -mtriple=x86_64-pc-win32-coff -passes=pgo-instr-gen -do-comdat-renaming=true -S | FileCheck --check-prefixes COMMON,COFFONLY %s ; Rename Comdat group and its function. $f = comdat any Index: test/Transforms/PGOProfile/indirect_call_profile.ll =================================================================== --- test/Transforms/PGOProfile/indirect_call_profile.ll +++ test/Transforms/PGOProfile/indirect_call_profile.ll @@ -54,7 +54,7 @@ } ; Test that comdat function's address is recorded. -; LOWER: @__profd_foo3.[[FOO3_HASH:[0-9]+]] = linkonce_odr{{.*}}@foo3.[[FOO3_HASH]] +; LOWER: @__profd_foo3.[[FOO3_HASH:[0-9]+]] = linkonce_odr{{.*}}@__profc_foo3.[[FOO3_HASH]] ; Function Attrs: nounwind uwtable define linkonce_odr i32 @foo3() comdat { ret i32 1