diff --git a/include/llvm/ProfileData/InstrProf.h b/include/llvm/ProfileData/InstrProf.h index c7e558efa3d..094f3af005d 100644 --- a/include/llvm/ProfileData/InstrProf.h +++ b/include/llvm/ProfileData/InstrProf.h @@ -230,15 +230,6 @@ class InstrProfSymtab; /// 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" diff --git a/lib/ProfileData/InstrProf.cpp b/lib/ProfileData/InstrProf.cpp index 74acd9e5e20..77c6ffc9c25 100644 --- a/lib/ProfileData/InstrProf.cpp +++ b/lib/ProfileData/InstrProf.cpp @@ -811,47 +811,4 @@ bool needsComdatForCounter(const Function &F, const Module &M) { 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 diff --git a/lib/Transforms/Instrumentation/InstrProfiling.cpp b/lib/Transforms/Instrumentation/InstrProfiling.cpp index adea7e77244..8da3e31200f 100644 --- a/lib/Transforms/Instrumentation/InstrProfiling.cpp +++ b/lib/Transforms/Instrumentation/InstrProfiling.cpp @@ -32,11 +32,6 @@ cl::opt DoNameCompression("enable-name-compression", 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"), @@ -277,16 +272,7 @@ void InstrProfiling::lowerCoverageData(GlobalVariable *CoverageNamesVar) { static std::string getVarName(InstrProfIncrementInst *Inc, StringRef Prefix) { StringRef NamePrefix = getInstrProfNameVarPrefix(); StringRef Name = Inc->getName()->getName().substr(NamePrefix.size()); - 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(); + return (Prefix + Name).str(); } static inline bool shouldRecordFunctionAddr(Function *F) { diff --git a/lib/Transforms/Instrumentation/PGOInstrumentation.cpp b/lib/Transforms/Instrumentation/PGOInstrumentation.cpp index ffebd42795d..28f4f7ea145 100644 --- a/lib/Transforms/Instrumentation/PGOInstrumentation.cpp +++ b/lib/Transforms/Instrumentation/PGOInstrumentation.cpp @@ -119,7 +119,7 @@ static cl::opt MaxNumAnnotations( // 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(false), cl::Hidden, + "do-comdat-renaming", cl::init(true), cl::Hidden, cl::desc("Append function hash to the name of COMDAT function to avoid " "function hash mismatch due to the preinliner")); @@ -407,8 +407,20 @@ void FuncPGOInstrumentation::computeCFGHash() { static bool canRenameComdat( Function &F, std::unordered_multimap &ComdatMembers) { - if (!DoComdatRenaming || !canRenameComdatFunc(F, true)) + 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; + } // FIXME: Current only handle those Comdat groups that only containing one // function and function aliases. diff --git a/test/Transforms/PGOProfile/Inputs/multiple_hash_profile.proftext b/test/Transforms/PGOProfile/Inputs/multiple_hash_profile.proftext deleted file mode 100644 index 5bf67fb2bfa..00000000000 --- a/test/Transforms/PGOProfile/Inputs/multiple_hash_profile.proftext +++ /dev/null @@ -1,36 +0,0 @@ -# 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 - diff --git a/test/Transforms/PGOProfile/comdat_internal.ll b/test/Transforms/PGOProfile/comdat_internal.ll index 7df6f91fe72..25dafbea103 100644 --- a/test/Transforms/PGOProfile/comdat_internal.ll +++ b/test/Transforms/PGOProfile/comdat_internal.ll @@ -4,17 +4,17 @@ target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128" target triple = "x86_64-unknown-linux-gnu" $foo = comdat any -; CHECK: $foo = comdat any +; CHECK: $foo.[[FOO_HASH:[0-9]+]] = comdat any ; CHECK: $__llvm_profile_raw_version = comdat any -; CHECK: $__profv__stdin__foo.[[FOO_HASH:[0-9]+]] = comdat any +; CHECK: $__profv__stdin__foo.[[FOO_HASH]] = comdat any @bar = global i32 ()* @foo, align 8 ; CHECK: @__llvm_profile_raw_version = constant i64 {{[0-9]+}}, comdat -; CHECK: @__profn__stdin__foo = private constant [11 x i8] c":foo" +; CHECK: @__profn__stdin__foo.[[FOO_HASH]] = private constant [23 x i8] c":foo.[[FOO_HASH]]" ; 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 -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: @__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-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 diff --git a/test/Transforms/PGOProfile/comdat_rename.ll b/test/Transforms/PGOProfile/comdat_rename.ll index eb9ddb4a1ce..b69c802093b 100644 --- a/test/Transforms/PGOProfile/comdat_rename.ll +++ b/test/Transforms/PGOProfile/comdat_rename.ll @@ -1,7 +1,7 @@ -; 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 +; 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 ; Rename Comdat group and its function. $f = comdat any @@ -38,10 +38,22 @@ define linkonce void @tf2() comdat($tf) { 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() { diff --git a/test/Transforms/PGOProfile/indirect_call_profile.ll b/test/Transforms/PGOProfile/indirect_call_profile.ll index e1f499c08a7..409c29ef872 100644 --- a/test/Transforms/PGOProfile/indirect_call_profile.ll +++ b/test/Transforms/PGOProfile/indirect_call_profile.ll @@ -54,7 +54,7 @@ bb11: ; preds = %bb2 } ; Test that comdat function's address is recorded. -; LOWER: @__profd_foo3.[[FOO3_HASH:[0-9]+]] = linkonce_odr{{.*}}@__profc_foo3.[[FOO3_HASH]] +; LOWER: @__profd_foo3.[[FOO3_HASH:[0-9]+]] = linkonce_odr{{.*}}@foo3.[[FOO3_HASH]] ; Function Attrs: nounwind uwtable define linkonce_odr i32 @foo3() comdat { ret i32 1 diff --git a/test/Transforms/PGOProfile/multiple_hash_profile.ll b/test/Transforms/PGOProfile/multiple_hash_profile.ll deleted file mode 100644 index f4041830f8f..00000000000 --- a/test/Transforms/PGOProfile/multiple_hash_profile.ll +++ /dev/null @@ -1,36 +0,0 @@ -; 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 -} - -