Index: include/llvm/ProfileData/InstrProf.h =================================================================== --- include/llvm/ProfileData/InstrProf.h +++ include/llvm/ProfileData/InstrProf.h @@ -230,6 +230,15 @@ /// 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. Instances of the same +/// comdat function may have different control flows thus can not share the +/// same counter variable. +bool canRenameComdatFunc(const Function &F, bool CheckAddressTaken = false); + 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,47 @@ 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 canRenameComdatFunc(const Function &F, bool CheckAddressTaken) { + if (F.getName().empty()) + return false; + if (!needsComdatForCounter(F, *(F.getParent()))) + return false; + // Unsafe to rename the address-taken function (which can be used in + // function comparison). + if (CheckAddressTaken && F.hasAddressTaken()) + 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,11 @@ cl::desc("Enable name string compression"), cl::init(true)); +cl::opt DoHashBasedCounterSplit( + "hash-based-counter-split", + cl::desc("Rename counter variable of a comdat function based on cfg hash"), + cl::init(true)); + cl::opt ValueProfileStaticAlloc( "vp-static-alloc", cl::desc("Do static counter allocation for value profiler"), @@ -272,7 +277,16 @@ 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 (!DoHashBasedCounterSplit || !isIRPGOFlagSet(M) || + !canRenameComdatFunc(*F)) + return (Prefix + Name).str(); + uint64_t FuncHash = Inc->getHash()->getZExtValue(); + 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 (!DoComdatRenaming || !canRenameComdatFunc(F, true)) 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/Inputs/multiple_hash_profile.proftext =================================================================== --- /dev/null +++ test/Transforms/PGOProfile/Inputs/multiple_hash_profile.proftext @@ -0,0 +1,36 @@ +# IR level Instrumentation Flag +:ir +_Z3fooi +# Func Hash: +72057606922829823 +# Num Counters: +2 +# Counter Values: +18 +12 + +_Z3fooi +# Func Hash: +12884901887 +# Num Counters: +1 +# Counter Values: +0 + +_Z3bari +# Func Hash: +72057606922829823 +# Num Counters: +2 +# Counter Values: +0 +0 + +_Z4m2f1v +# Func Hash: +12884901887 +# Num Counters: +1 +# Counter Values: +1 + 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 @@ -38,22 +38,10 @@ ret void } -; Renaming Comdat with aliases. -$f_with_alias = comdat any -; COMMON: $f_with_alias.[[SINGLEBB_HASH]] = comdat any -@af = alias void (...), bitcast (void ()* @f_with_alias to void (...)*) -; COFFONLY: @af.[[SINGLEBB_HASH]] = alias void (...), bitcast (void ()* @f_with_alias.[[SINGLEBB_HASH]] to -; ELFONLY-DAG: @af.[[SINGLEBB_HASH]] = alias void (...), bitcast (void ()* @f_with_alias.[[SINGLEBB_HASH]] to -define linkonce_odr void @f_with_alias() comdat($f_with_alias) { - ret void -} - ; Rename AvailableExternallyLinkage functions ; ELFONLY-DAG: $aef.[[SINGLEBB_HASH]] = comdat any ; ELFONLY: @f = weak alias void (), void ()* @f.[[SINGLEBB_HASH]] -; ELFONLY: @f_with_alias = weak alias void (), void ()* @f_with_alias.[[SINGLEBB_HASH]] -; ELFONLY: @af = weak alias void (...), void (...)* @af.[[SINGLEBB_HASH]] ; ELFONLY: @aef = weak alias void (), void ()* @aef.[[SINGLEBB_HASH]] define available_externally void @aef() { 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 Index: test/Transforms/PGOProfile/multiple_hash_profile.ll =================================================================== --- /dev/null +++ test/Transforms/PGOProfile/multiple_hash_profile.ll @@ -0,0 +1,36 @@ +; RUN: llvm-profdata merge %S/Inputs/multiple_hash_profile.proftext -o %t.profdata +; RUN: opt < %s -pgo-instr-use -pgo-test-profile-file=%t.profdata -S | FileCheck %s +; RUN: opt < %s -passes=pgo-instr-use -pgo-test-profile-file=%t.profdata -S | FileCheck %s +target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128" +target triple = "x86_64-unknown-linux-gnu" + +$_Z3fooi = comdat any + +@g2 = local_unnamed_addr global i32 (i32)* null, align 8 + +define i32 @_Z3bari(i32 %i) { +entry: + %cmp = icmp sgt i32 %i, 2 + %mul = select i1 %cmp, i32 1, i32 %i + %retval.0 = mul nsw i32 %mul, %i + ret i32 %retval.0 +} + +define void @_Z4m2f1v() { +entry: + store i32 (i32)* @_Z3fooi, i32 (i32)** @g2, align 8 + ret void +} + +define linkonce_odr i32 @_Z3fooi(i32 %i) comdat { +entry: + %cmp.i = icmp sgt i32 %i, 2 + %mul.i = select i1 %cmp.i, i32 1, i32 %i +; CHECK: %mul.i = select i1 %cmp.i, i32 1, i32 %i +; CHECK-SAME !prof ![[BW:[0-9]+]] +; CHECK ![[BW]] = !{!"branch_weights", i32 12, i32 6} + %retval.0.i = mul nsw i32 %mul.i, %i + ret i32 %retval.0.i +} + +