llvm.org GIT mirror llvm / 2cf892f
[PGO] Turn off comdat renaming in IR PGO by default Summary: In IR PGO we append the function hash to comdat functions to avoid the potential hash mismatch. This turns out not legal in some cases: if the comdat function is address-taken and used in comparison. Renaming changes the semantic. This patch turns off comdat renaming by default. To alleviate the hash mismatch issue, we now rename the profile variable for comdat functions. Profile allows co-existing multiple versions of profiles with different hash value. The inlined copy will always has the correct profile counter. The out-of-line copy might not have the correct count. But we will not have the bogus mismatch warning. Reviewers: davidxl Subscribers: llvm-commits, xur Differential Revision: https://reviews.llvm.org/D28416 git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@291588 91177308-0d34-0410-b5e6-96231b3b80d8 Rong Xu 2 years ago
9 changed file(s) with 150 addition(s) and 36 deletion(s). Raw diff Collapse all Expand all
228228 /// the format described above. The substrings are separated by 0 or more zero
229229 /// bytes. This method decodes the string and populates the \c Symtab.
230230 Error readPGOFuncNameStrings(StringRef NameStrings, InstrProfSymtab &Symtab);
231
232 /// Check if INSTR_PROF_RAW_VERSION_VAR is defined. This global is only being
233 /// set in IR PGO compilation.
234 bool isIRPGOFlagSet(const Module *M);
235
236 /// Check if we can safely rename this Comdat function. Instances of the same
237 /// comdat function may have different control flows thus can not share the
238 /// same counter variable.
239 bool canRenameComdatFunc(const Function &F, bool CheckAddressTaken = false);
231240
232241 enum InstrProfValueKind : uint32_t {
233242 #define VALUE_PROF_KIND(Enumerator, Value) Enumerator = Value,
810810
811811 return true;
812812 }
813
814 // Check if INSTR_PROF_RAW_VERSION_VAR is defined.
815 bool isIRPGOFlagSet(const Module *M) {
816 auto IRInstrVar =
817 M->getNamedGlobal(INSTR_PROF_QUOTE(INSTR_PROF_RAW_VERSION_VAR));
818 if (!IRInstrVar || IRInstrVar->isDeclaration() ||
819 IRInstrVar->hasLocalLinkage())
820 return false;
821
822 // Check if the flag is set.
823 if (!IRInstrVar->hasInitializer())
824 return false;
825
826 const Constant *InitVal = IRInstrVar->getInitializer();
827 if (!InitVal)
828 return false;
829
830 return (dyn_cast(InitVal)->getZExtValue() &
831 VARIANT_MASK_IR_PROF) != 0;
832 }
833
834 // Check if we can safely rename this Comdat function.
835 bool canRenameComdatFunc(const Function &F, bool CheckAddressTaken) {
836 if (F.getName().empty())
837 return false;
838 if (!needsComdatForCounter(F, *(F.getParent())))
839 return false;
840 // Unsafe to rename the address-taken function (which can be used in
841 // function comparison).
842 if (CheckAddressTaken && F.hasAddressTaken())
843 return false;
844 // Only safe to do if this function may be discarded if it is not used
845 // in the compilation unit.
846 if (!GlobalValue::isDiscardableIfUnused(F.getLinkage()))
847 return false;
848
849 // For AvailableExternallyLinkage functions.
850 if (!F.hasComdat()) {
851 assert(F.getLinkage() == GlobalValue::AvailableExternallyLinkage);
852 return true;
853 }
854 return true;
855 }
813856 } // end namespace llvm
3030 cl::opt DoNameCompression("enable-name-compression",
3131 cl::desc("Enable name string compression"),
3232 cl::init(true));
33
34 cl::opt DoHashBasedCounterSplit(
35 "hash-based-counter-split",
36 cl::desc("Rename counter variable of a comdat function based on cfg hash"),
37 cl::init(true));
3338
3439 cl::opt ValueProfileStaticAlloc(
3540 "vp-static-alloc",
271276 static std::string getVarName(InstrProfIncrementInst *Inc, StringRef Prefix) {
272277 StringRef NamePrefix = getInstrProfNameVarPrefix();
273278 StringRef Name = Inc->getName()->getName().substr(NamePrefix.size());
274 return (Prefix + Name).str();
279 Function *F = Inc->getParent()->getParent();
280 Module *M = F->getParent();
281 if (!DoHashBasedCounterSplit || !isIRPGOFlagSet(M) ||
282 !canRenameComdatFunc(*F))
283 return (Prefix + Name).str();
284 uint64_t FuncHash = Inc->getHash()->getZExtValue();
285 SmallVector HashPostfix;
286 if (Name.endswith((Twine(".") + Twine(FuncHash)).toStringRef(HashPostfix)))
287 return (Prefix + Name).str();
288 return (Prefix + Name + "." + Twine(FuncHash)).str();
275289 }
276290
277291 static inline bool shouldRecordFunctionAddr(Function *F) {
118118 // Command line option to control appending FunctionHash to the name of a COMDAT
119119 // function. This is to avoid the hash mismatch caused by the preinliner.
120120 static cl::opt DoComdatRenaming(
121 "do-comdat-renaming", cl::init(true), cl::Hidden,
121 "do-comdat-renaming", cl::init(false), cl::Hidden,
122122 cl::desc("Append function hash to the name of COMDAT function to avoid "
123123 "function hash mismatch due to the preinliner"));
124124
406406 static bool canRenameComdat(
407407 Function &F,
408408 std::unordered_multimap &ComdatMembers) {
409 if (F.getName().empty())
409 if (!DoComdatRenaming || !canRenameComdatFunc(F, true))
410410 return false;
411 if (!needsComdatForCounter(F, *(F.getParent())))
412 return false;
413 // Only safe to do if this function may be discarded if it is not used
414 // in the compilation unit.
415 if (!GlobalValue::isDiscardableIfUnused(F.getLinkage()))
416 return false;
417
418 // For AvailableExternallyLinkage functions.
419 if (!F.hasComdat()) {
420 assert(F.getLinkage() == GlobalValue::AvailableExternallyLinkage);
421 return true;
422 }
423411
424412 // FIXME: Current only handle those Comdat groups that only containing one
425413 // function and function aliases.
0 # IR level Instrumentation Flag
1 :ir
2 _Z3fooi
3 # Func Hash:
4 72057606922829823
5 # Num Counters:
6 2
7 # Counter Values:
8 18
9 12
10
11 _Z3fooi
12 # Func Hash:
13 12884901887
14 # Num Counters:
15 1
16 # Counter Values:
17 0
18
19 _Z3bari
20 # Func Hash:
21 72057606922829823
22 # Num Counters:
23 2
24 # Counter Values:
25 0
26 0
27
28 _Z4m2f1v
29 # Func Hash:
30 12884901887
31 # Num Counters:
32 1
33 # Counter Values:
34 1
35
33 target triple = "x86_64-unknown-linux-gnu"
44
55 $foo = comdat any
6 ; CHECK: $foo.[[FOO_HASH:[0-9]+]] = comdat any
6 ; CHECK: $foo = comdat any
77
88 ; CHECK: $__llvm_profile_raw_version = comdat any
9 ; CHECK: $__profv__stdin__foo.[[FOO_HASH]] = comdat any
9 ; CHECK: $__profv__stdin__foo.[[FOO_HASH:[0-9]+]] = comdat any
1010
1111 @bar = global i32 ()* @foo, align 8
1212
1313 ; CHECK: @__llvm_profile_raw_version = constant i64 {{[0-9]+}}, comdat
14 ; CHECK: @__profn__stdin__foo.[[FOO_HASH]] = private constant [23 x i8] c":foo.[[FOO_HASH]]"
14 ; CHECK: @__profn__stdin__foo = private constant [11 x i8] c":foo"
1515 ; CHECK: @__profc__stdin__foo.[[FOO_HASH]] = private global [1 x i64] zeroinitializer, section "__llvm_prf_cnts", comdat($__profv__stdin__foo.[[FOO_HASH]]), align 8
16 ; 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
16 ; 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
1717 ; CHECK-NOT: bitcast (i32 ()* @foo to i8*)
1818 ; CHECK-SAME: , i8* null, i32 1, [1 x i16] zeroinitializer }, section "__llvm_prf_data", comdat($__profv__stdin__foo.[[FOO_HASH]]), align 8
1919 ; CHECK: @__llvm_prf_nm
None ; RUN: opt < %s -mtriple=x86_64-unknown-linux -pgo-instr-gen -S | FileCheck --check-prefixes COMMON,ELFONLY %s
1 ; RUN: opt < %s -mtriple=x86_64-unknown-linux -passes=pgo-instr-gen -S | FileCheck --check-prefixes COMMON,ELFONLY %s
2 ; RUN: opt < %s -mtriple=x86_64-pc-win32-coff -pgo-instr-gen -S | FileCheck --check-prefixes COMMON,COFFONLY %s
3 ; RUN: opt < %s -mtriple=x86_64-pc-win32-coff -passes=pgo-instr-gen -S | FileCheck --check-prefixes COMMON,COFFONLY %s
0 ; RUN: opt < %s -mtriple=x86_64-unknown-linux -pgo-instr-gen -do-comdat-renaming=true -S | FileCheck --check-prefixes COMMON,ELFONLY %s
1 ; RUN: opt < %s -mtriple=x86_64-unknown-linux -passes=pgo-instr-gen -do-comdat-renaming=true -S | FileCheck --check-prefixes COMMON,ELFONLY %s
2 ; RUN: opt < %s -mtriple=x86_64-pc-win32-coff -pgo-instr-gen -do-comdat-renaming=true -S | FileCheck --check-prefixes COMMON,COFFONLY %s
3 ; RUN: opt < %s -mtriple=x86_64-pc-win32-coff -passes=pgo-instr-gen -do-comdat-renaming=true -S | FileCheck --check-prefixes COMMON,COFFONLY %s
44
55 ; Rename Comdat group and its function.
66 $f = comdat any
3737 ret void
3838 }
3939
40 ; Renaming Comdat with aliases.
41 $f_with_alias = comdat any
42 ; COMMON: $f_with_alias.[[SINGLEBB_HASH]] = comdat any
43 @af = alias void (...), bitcast (void ()* @f_with_alias to void (...)*)
44 ; COFFONLY: @af.[[SINGLEBB_HASH]] = alias void (...), bitcast (void ()* @f_with_alias.[[SINGLEBB_HASH]] to
45 ; ELFONLY-DAG: @af.[[SINGLEBB_HASH]] = alias void (...), bitcast (void ()* @f_with_alias.[[SINGLEBB_HASH]] to
46 define linkonce_odr void @f_with_alias() comdat($f_with_alias) {
47 ret void
48 }
49
5040 ; Rename AvailableExternallyLinkage functions
5141 ; ELFONLY-DAG: $aef.[[SINGLEBB_HASH]] = comdat any
5242
5343 ; ELFONLY: @f = weak alias void (), void ()* @f.[[SINGLEBB_HASH]]
54 ; ELFONLY: @f_with_alias = weak alias void (), void ()* @f_with_alias.[[SINGLEBB_HASH]]
55 ; ELFONLY: @af = weak alias void (...), void (...)* @af.[[SINGLEBB_HASH]]
5644 ; ELFONLY: @aef = weak alias void (), void ()* @aef.[[SINGLEBB_HASH]]
5745
5846 define available_externally void @aef() {
5353 }
5454
5555 ; Test that comdat function's address is recorded.
56 ; LOWER: @__profd_foo3.[[FOO3_HASH:[0-9]+]] = linkonce_odr{{.*}}@foo3.[[FOO3_HASH]]
56 ; LOWER: @__profd_foo3.[[FOO3_HASH:[0-9]+]] = linkonce_odr{{.*}}@__profc_foo3.[[FOO3_HASH]]
5757 ; Function Attrs: nounwind uwtable
5858 define linkonce_odr i32 @foo3() comdat {
5959 ret i32 1
0 ; RUN: llvm-profdata merge %S/Inputs/multiple_hash_profile.proftext -o %t.profdata
1 ; RUN: opt < %s -pgo-instr-use -pgo-test-profile-file=%t.profdata -S | FileCheck %s
2 ; RUN: opt < %s -passes=pgo-instr-use -pgo-test-profile-file=%t.profdata -S | FileCheck %s
3 target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
4 target triple = "x86_64-unknown-linux-gnu"
5
6 $_Z3fooi = comdat any
7
8 @g2 = local_unnamed_addr global i32 (i32)* null, align 8
9
10 define i32 @_Z3bari(i32 %i) {
11 entry:
12 %cmp = icmp sgt i32 %i, 2
13 %mul = select i1 %cmp, i32 1, i32 %i
14 %retval.0 = mul nsw i32 %mul, %i
15 ret i32 %retval.0
16 }
17
18 define void @_Z4m2f1v() {
19 entry:
20 store i32 (i32)* @_Z3fooi, i32 (i32)** @g2, align 8
21 ret void
22 }
23
24 define linkonce_odr i32 @_Z3fooi(i32 %i) comdat {
25 entry:
26 %cmp.i = icmp sgt i32 %i, 2
27 %mul.i = select i1 %cmp.i, i32 1, i32 %i
28 ; CHECK: %mul.i = select i1 %cmp.i, i32 1, i32 %i
29 ; CHECK-SAME !prof ![[BW:[0-9]+]]
30 ; CHECK ![[BW]] = !{!"branch_weights", i32 12, i32 6}
31 %retval.0.i = mul nsw i32 %mul.i, %i
32 ret i32 %retval.0.i
33 }
34
35