From bc4335458b054f25f9e8e1fd1148375d80725d83 Mon Sep 17 00:00:00 2001 From: mingmingl Date: Mon, 30 Sep 2024 14:16:12 -0700 Subject: [PATCH 1/5] [TypeProf][PGO]Support specification of vtables for which type profiles are known to be unrepresentative --- .../Instrumentation/IndirectCallPromotion.cpp | 46 +++++++++++++++++-- .../Transforms/PGOProfile/icp_vtable_cmp.ll | 1 + 2 files changed, 43 insertions(+), 4 deletions(-) diff --git a/llvm/lib/Transforms/Instrumentation/IndirectCallPromotion.cpp b/llvm/lib/Transforms/Instrumentation/IndirectCallPromotion.cpp index fbed593ab3aa7..6031bb8c05fa3 100644 --- a/llvm/lib/Transforms/Instrumentation/IndirectCallPromotion.cpp +++ b/llvm/lib/Transforms/Instrumentation/IndirectCallPromotion.cpp @@ -132,6 +132,11 @@ static cl::opt ICPMaxNumVTableLastCandidate( "icp-max-num-vtable-last-candidate", cl::init(1), cl::Hidden, cl::desc("The maximum number of vtable for the last candidate.")); +static cl::opt ICPKnownUnrepresentativeVTables( + "icp-known-unrepresentative-vtables", cl::init(""), cl::Hidden, + cl::desc("A comma-separated list of mangled vtable names for which instrumented + profiles are not representative. For instance, the instantiated class is arch or micro-arch specific, while instrumented profiles are collected on one arch.")); + namespace { // The key is a vtable global variable, and the value is a map. @@ -316,6 +321,8 @@ class IndirectCallPromoter { OptimizationRemarkEmitter &ORE; + const DenseSet &KnownUnrepresentativeBaseTypes; + // A struct that records the direct target and it's call count. struct PromotionCandidate { Function *const TargetFunction; @@ -391,10 +398,12 @@ class IndirectCallPromoter { Function &Func, Module &M, InstrProfSymtab *Symtab, bool SamplePGO, const VirtualCallSiteTypeInfoMap &VirtualCSInfo, VTableAddressPointOffsetValMap &VTableAddressPointOffsetVal, + DenseSet &KnownUnrepresentativeTypes, OptimizationRemarkEmitter &ORE) : F(Func), M(M), Symtab(Symtab), SamplePGO(SamplePGO), VirtualCSInfo(VirtualCSInfo), - VTableAddressPointOffsetVal(VTableAddressPointOffsetVal), ORE(ORE) {} + VTableAddressPointOffsetVal(VTableAddressPointOffsetVal), ORE(ORE), + KnownUnrepresentativeBaseTypes(KnownUnrepresentativeTypes) {} IndirectCallPromoter(const IndirectCallPromoter &) = delete; IndirectCallPromoter &operator=(const IndirectCallPromoter &) = delete; @@ -851,8 +860,26 @@ bool IndirectCallPromoter::isProfitableToCompareVTables( LLVM_DEBUG(dbgs() << "\n"); uint64_t CandidateVTableCount = 0; - for (auto &[GUID, Count] : VTableGUIDAndCounts) + SmallVector Types; + for (auto &[GUID, Count] : VTableGUIDAndCounts) { CandidateVTableCount += Count; + auto *VTableVar = Symtab->getGlobalVariable(GUID); + + assert(VTableVar && + "VTableVar must exist for GUID in VTableGUIDAndCounts"); + + Types.clear(); + VTableVar->getMetadata(LLVMContext::MD_type, Types); + + for (auto *Type : Types) + if (auto *TypeId = dyn_cast(Type->getOperand(1).get())) + if (KnownUnrepresentativeBaseTypes.contains(TypeId->getString())) { + LLVM_DEBUG(dbgs() + << " vtable profiles are known to be " + "unrepresentative. Bail out vtable comparison.") + return false; + } + } if (CandidateVTableCount < Candidate.Count * ICPVTablePercentageThreshold) { LLVM_DEBUG( @@ -956,9 +983,19 @@ static bool promoteIndirectCalls(Module &M, ProfileSummaryInfo *PSI, bool InLTO, bool Changed = false; VirtualCallSiteTypeInfoMap VirtualCSInfo; - if (EnableVTableProfileUse) + DenseSet KnownUnrepresentativeTypeSet; + + if (EnableVTableProfileUse) { computeVirtualCallSiteTypeInfoMap(M, MAM, VirtualCSInfo); + SmallVector KnownUnrepresentativeTypes; + llvm::SplitString(ICPKnownUnrepresentativeVTables, + KnownUnrepresentativeTypes); + + for (const StringRef Str : KnownUnrepresentativeTypes) + KnownUnrepresentativeTypeSet.insert(Str); + } + // VTableAddressPointOffsetVal stores the vtable address points. The vtable // address point of a given is static (doesn't // change after being computed once). @@ -977,7 +1014,8 @@ static bool promoteIndirectCalls(Module &M, ProfileSummaryInfo *PSI, bool InLTO, auto &ORE = FAM.getResult(F); IndirectCallPromoter CallPromoter(F, M, &Symtab, SamplePGO, VirtualCSInfo, - VTableAddressPointOffsetVal, ORE); + VTableAddressPointOffsetVal, + KnownUnrepresentativeTypeSet, ORE); bool FuncChanged = CallPromoter.processFunction(PSI); if (ICPDUMPAFTER && FuncChanged) { LLVM_DEBUG(dbgs() << "\n== IR Dump After =="; F.print(dbgs())); diff --git a/llvm/test/Transforms/PGOProfile/icp_vtable_cmp.ll b/llvm/test/Transforms/PGOProfile/icp_vtable_cmp.ll index b6afce3d7c6d5..bbae25787a05c 100644 --- a/llvm/test/Transforms/PGOProfile/icp_vtable_cmp.ll +++ b/llvm/test/Transforms/PGOProfile/icp_vtable_cmp.ll @@ -2,6 +2,7 @@ ; RUN: opt < %s -passes='pgo-icall-prom' -pass-remarks=pgo-icall-prom -enable-vtable-profile-use -icp-max-num-vtable-last-candidate=2 -S 2>&1 | FileCheck %s --check-prefixes=VTABLE-COMMON,VTABLE-CMP ; RUN: opt < %s -passes='pgo-icall-prom' -pass-remarks=pgo-icall-prom -enable-vtable-profile-use -icp-max-num-vtable-last-candidate=1 -S 2>&1 | FileCheck %s --check-prefixes=VTABLE-COMMON,FUNC-CMP +; RUN: opt < %s -passes='pgo-icall-prom' -pass-remarks=pgo-icall-prom -enable-vtable-profile-use -icp-max-num-vtable-last-candidate=1 -icp-known-unrepresentative-vtables='Base1,Derived3' -S 2>&1 | FileCheck %s --check-prefixes=VTABLE-COMMON,FUNC-CMP 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" From 26956410ea50e3b5e82fb6a50dd010495d92d104 Mon Sep 17 00:00:00 2001 From: mingmingl Date: Mon, 30 Sep 2024 17:54:05 -0700 Subject: [PATCH 2/5] resolve review feedback --- llvm/include/llvm/Support/CommandLine.h | 20 ++++++++++ llvm/lib/Support/CommandLine.cpp | 20 ++++++++++ .../Instrumentation/IndirectCallPromotion.cpp | 37 +++++++------------ .../Transforms/PGOProfile/icp_vtable_cmp.ll | 2 +- 4 files changed, 54 insertions(+), 25 deletions(-) diff --git a/llvm/include/llvm/Support/CommandLine.h b/llvm/include/llvm/Support/CommandLine.h index 5d60bb64bbb20..7454db1973ad4 100644 --- a/llvm/include/llvm/Support/CommandLine.h +++ b/llvm/include/llvm/Support/CommandLine.h @@ -1208,6 +1208,26 @@ template <> class parser : public basic_parser { void anchor() override; }; +//-------------------------------------------------- + +extern template class basic_parser>; + +template <> +class parser> : public basic_parser> { +public: + parser(Option &O) : basic_parser(O) {} + + // Return true on error. + bool parse(Option &, StringRef, StringRef Arg, DenseSet &Val); + + StringRef getValueName() const override { return "DenseSet"; } + + void printOptionDiff(const Option &O, const DenseSet &V, + OptVal Default, size_t GlobalWidth) const; + + void anchor() override; +}; + //-------------------------------------------------- // This collection of wrappers is the intermediary between class opt and class // parser to handle all the template nastiness. diff --git a/llvm/lib/Support/CommandLine.cpp b/llvm/lib/Support/CommandLine.cpp index e34a770b1b53e..7f22c82f6fa9d 100644 --- a/llvm/lib/Support/CommandLine.cpp +++ b/llvm/lib/Support/CommandLine.cpp @@ -66,6 +66,7 @@ template class basic_parser; template class basic_parser; template class basic_parser; template class basic_parser; +template class basic_parser>; template class opt; template class opt; @@ -93,6 +94,7 @@ void parser::anchor() {} void parser::anchor() {} void parser::anchor() {} void parser::anchor() {} +void parser>::anchor() {} //===----------------------------------------------------------------------===// @@ -2060,6 +2062,24 @@ bool parser::parse(Option &O, StringRef ArgName, StringRef Arg, return false; } +// parser implementation +// +void parser>::printOptionDiff( + const Option &O, const DenseSet &V, + OptionValue> D, size_t GlobalWidth) const {} + +bool parser>::parse(Option &O, StringRef ArgName, + StringRef Arg, + DenseSet &Val) { + SmallVector StrRefs; + llvm::SplitString(Arg, StrRefs, ","); + for (const StringRef StrRef : StrRefs) + if (!Val.insert(StrRef).second) + return O.error(Arg + " has duplicated strings!"); + + return false; +} + // generic_parser_base implementation // diff --git a/llvm/lib/Transforms/Instrumentation/IndirectCallPromotion.cpp b/llvm/lib/Transforms/Instrumentation/IndirectCallPromotion.cpp index 6031bb8c05fa3..8a8baff80eba0 100644 --- a/llvm/lib/Transforms/Instrumentation/IndirectCallPromotion.cpp +++ b/llvm/lib/Transforms/Instrumentation/IndirectCallPromotion.cpp @@ -132,10 +132,13 @@ static cl::opt ICPMaxNumVTableLastCandidate( "icp-max-num-vtable-last-candidate", cl::init(1), cl::Hidden, cl::desc("The maximum number of vtable for the last candidate.")); -static cl::opt ICPKnownUnrepresentativeVTables( - "icp-known-unrepresentative-vtables", cl::init(""), cl::Hidden, - cl::desc("A comma-separated list of mangled vtable names for which instrumented - profiles are not representative. For instance, the instantiated class is arch or micro-arch specific, while instrumented profiles are collected on one arch.")); +static cl::opt> ICPIgnoredBaseTypes( + "icp-ignored-base-types", cl::Hidden, cl::init(DenseSet()), + cl::desc("A comma-separated list of mangled vtable names. Classes " + "specified by the vtables and their derived ones will not be " + "vtable-ICP'ed. Useful when the profiled types and actual types " + "in the optimized binary could be different due to profiling " + "limitations.")); namespace { @@ -321,8 +324,6 @@ class IndirectCallPromoter { OptimizationRemarkEmitter &ORE; - const DenseSet &KnownUnrepresentativeBaseTypes; - // A struct that records the direct target and it's call count. struct PromotionCandidate { Function *const TargetFunction; @@ -398,12 +399,10 @@ class IndirectCallPromoter { Function &Func, Module &M, InstrProfSymtab *Symtab, bool SamplePGO, const VirtualCallSiteTypeInfoMap &VirtualCSInfo, VTableAddressPointOffsetValMap &VTableAddressPointOffsetVal, - DenseSet &KnownUnrepresentativeTypes, OptimizationRemarkEmitter &ORE) : F(Func), M(M), Symtab(Symtab), SamplePGO(SamplePGO), VirtualCSInfo(VirtualCSInfo), - VTableAddressPointOffsetVal(VTableAddressPointOffsetVal), ORE(ORE), - KnownUnrepresentativeBaseTypes(KnownUnrepresentativeTypes) {} + VTableAddressPointOffsetVal(VTableAddressPointOffsetVal), ORE(ORE) {} IndirectCallPromoter(const IndirectCallPromoter &) = delete; IndirectCallPromoter &operator=(const IndirectCallPromoter &) = delete; @@ -871,12 +870,13 @@ bool IndirectCallPromoter::isProfitableToCompareVTables( Types.clear(); VTableVar->getMetadata(LLVMContext::MD_type, Types); + const DenseSet &VTableSet = ICPIgnoredBaseTypes.getValue(); for (auto *Type : Types) if (auto *TypeId = dyn_cast(Type->getOperand(1).get())) - if (KnownUnrepresentativeBaseTypes.contains(TypeId->getString())) { + if (VTableSet.contains(TypeId->getString().str())) { LLVM_DEBUG(dbgs() << " vtable profiles are known to be " - "unrepresentative. Bail out vtable comparison.") + "unrepresentative. Bail out vtable comparison."); return false; } } @@ -983,19 +983,9 @@ static bool promoteIndirectCalls(Module &M, ProfileSummaryInfo *PSI, bool InLTO, bool Changed = false; VirtualCallSiteTypeInfoMap VirtualCSInfo; - DenseSet KnownUnrepresentativeTypeSet; - - if (EnableVTableProfileUse) { + if (EnableVTableProfileUse) computeVirtualCallSiteTypeInfoMap(M, MAM, VirtualCSInfo); - SmallVector KnownUnrepresentativeTypes; - llvm::SplitString(ICPKnownUnrepresentativeVTables, - KnownUnrepresentativeTypes); - - for (const StringRef Str : KnownUnrepresentativeTypes) - KnownUnrepresentativeTypeSet.insert(Str); - } - // VTableAddressPointOffsetVal stores the vtable address points. The vtable // address point of a given is static (doesn't // change after being computed once). @@ -1014,8 +1004,7 @@ static bool promoteIndirectCalls(Module &M, ProfileSummaryInfo *PSI, bool InLTO, auto &ORE = FAM.getResult(F); IndirectCallPromoter CallPromoter(F, M, &Symtab, SamplePGO, VirtualCSInfo, - VTableAddressPointOffsetVal, - KnownUnrepresentativeTypeSet, ORE); + VTableAddressPointOffsetVal, ORE); bool FuncChanged = CallPromoter.processFunction(PSI); if (ICPDUMPAFTER && FuncChanged) { LLVM_DEBUG(dbgs() << "\n== IR Dump After =="; F.print(dbgs())); diff --git a/llvm/test/Transforms/PGOProfile/icp_vtable_cmp.ll b/llvm/test/Transforms/PGOProfile/icp_vtable_cmp.ll index bbae25787a05c..f3a43a830fa12 100644 --- a/llvm/test/Transforms/PGOProfile/icp_vtable_cmp.ll +++ b/llvm/test/Transforms/PGOProfile/icp_vtable_cmp.ll @@ -2,7 +2,7 @@ ; RUN: opt < %s -passes='pgo-icall-prom' -pass-remarks=pgo-icall-prom -enable-vtable-profile-use -icp-max-num-vtable-last-candidate=2 -S 2>&1 | FileCheck %s --check-prefixes=VTABLE-COMMON,VTABLE-CMP ; RUN: opt < %s -passes='pgo-icall-prom' -pass-remarks=pgo-icall-prom -enable-vtable-profile-use -icp-max-num-vtable-last-candidate=1 -S 2>&1 | FileCheck %s --check-prefixes=VTABLE-COMMON,FUNC-CMP -; RUN: opt < %s -passes='pgo-icall-prom' -pass-remarks=pgo-icall-prom -enable-vtable-profile-use -icp-max-num-vtable-last-candidate=1 -icp-known-unrepresentative-vtables='Base1,Derived3' -S 2>&1 | FileCheck %s --check-prefixes=VTABLE-COMMON,FUNC-CMP +; RUN: opt < %s -passes='pgo-icall-prom' -pass-remarks=pgo-icall-prom -enable-vtable-profile-use -icp-max-num-vtable-last-candidate=1 -icp-ignored-base-types='Base1,Derived3' -S 2>&1 | FileCheck %s --check-prefixes=VTABLE-COMMON,FUNC-CMP 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" From f14c6d55a227d783875794789433738f998c1c48 Mon Sep 17 00:00:00 2001 From: mingmingl Date: Tue, 1 Oct 2024 11:27:20 -0700 Subject: [PATCH 3/5] 1. use cl::list for option type 2. Update `-icp-ignored-base-types` with one base type since this is sufficient --- llvm/include/llvm/Support/CommandLine.h | 20 --------- llvm/lib/Support/CommandLine.cpp | 20 --------- .../Instrumentation/IndirectCallPromotion.cpp | 44 ++++++++++++------- .../Transforms/PGOProfile/icp_vtable_cmp.ll | 2 +- 4 files changed, 28 insertions(+), 58 deletions(-) diff --git a/llvm/include/llvm/Support/CommandLine.h b/llvm/include/llvm/Support/CommandLine.h index 7454db1973ad4..5d60bb64bbb20 100644 --- a/llvm/include/llvm/Support/CommandLine.h +++ b/llvm/include/llvm/Support/CommandLine.h @@ -1208,26 +1208,6 @@ template <> class parser : public basic_parser { void anchor() override; }; -//-------------------------------------------------- - -extern template class basic_parser>; - -template <> -class parser> : public basic_parser> { -public: - parser(Option &O) : basic_parser(O) {} - - // Return true on error. - bool parse(Option &, StringRef, StringRef Arg, DenseSet &Val); - - StringRef getValueName() const override { return "DenseSet"; } - - void printOptionDiff(const Option &O, const DenseSet &V, - OptVal Default, size_t GlobalWidth) const; - - void anchor() override; -}; - //-------------------------------------------------- // This collection of wrappers is the intermediary between class opt and class // parser to handle all the template nastiness. diff --git a/llvm/lib/Support/CommandLine.cpp b/llvm/lib/Support/CommandLine.cpp index 7f22c82f6fa9d..e34a770b1b53e 100644 --- a/llvm/lib/Support/CommandLine.cpp +++ b/llvm/lib/Support/CommandLine.cpp @@ -66,7 +66,6 @@ template class basic_parser; template class basic_parser; template class basic_parser; template class basic_parser; -template class basic_parser>; template class opt; template class opt; @@ -94,7 +93,6 @@ void parser::anchor() {} void parser::anchor() {} void parser::anchor() {} void parser::anchor() {} -void parser>::anchor() {} //===----------------------------------------------------------------------===// @@ -2062,24 +2060,6 @@ bool parser::parse(Option &O, StringRef ArgName, StringRef Arg, return false; } -// parser implementation -// -void parser>::printOptionDiff( - const Option &O, const DenseSet &V, - OptionValue> D, size_t GlobalWidth) const {} - -bool parser>::parse(Option &O, StringRef ArgName, - StringRef Arg, - DenseSet &Val) { - SmallVector StrRefs; - llvm::SplitString(Arg, StrRefs, ","); - for (const StringRef StrRef : StrRefs) - if (!Val.insert(StrRef).second) - return O.error(Arg + " has duplicated strings!"); - - return false; -} - // generic_parser_base implementation // diff --git a/llvm/lib/Transforms/Instrumentation/IndirectCallPromotion.cpp b/llvm/lib/Transforms/Instrumentation/IndirectCallPromotion.cpp index 8a8baff80eba0..83e717b4bcd2e 100644 --- a/llvm/lib/Transforms/Instrumentation/IndirectCallPromotion.cpp +++ b/llvm/lib/Transforms/Instrumentation/IndirectCallPromotion.cpp @@ -132,13 +132,14 @@ static cl::opt ICPMaxNumVTableLastCandidate( "icp-max-num-vtable-last-candidate", cl::init(1), cl::Hidden, cl::desc("The maximum number of vtable for the last candidate.")); -static cl::opt> ICPIgnoredBaseTypes( - "icp-ignored-base-types", cl::Hidden, cl::init(DenseSet()), - cl::desc("A comma-separated list of mangled vtable names. Classes " - "specified by the vtables and their derived ones will not be " - "vtable-ICP'ed. Useful when the profiled types and actual types " - "in the optimized binary could be different due to profiling " - "limitations.")); +static cl::list ICPIgnoredBaseTypes( + "icp-ignored-base-types", cl::Hidden, + cl::desc( + "A list of mangled vtable names. Classes specified by the vtables " + "and their derived ones will not be vtable-ICP'ed. Useful when the " + "profiled types and actual types in the optimized binary could be " + "different due to profiling " + "limitations.")); namespace { @@ -324,6 +325,8 @@ class IndirectCallPromoter { OptimizationRemarkEmitter &ORE; + const DenseSet &IgnoredBaseTypes; + // A struct that records the direct target and it's call count. struct PromotionCandidate { Function *const TargetFunction; @@ -399,10 +402,12 @@ class IndirectCallPromoter { Function &Func, Module &M, InstrProfSymtab *Symtab, bool SamplePGO, const VirtualCallSiteTypeInfoMap &VirtualCSInfo, VTableAddressPointOffsetValMap &VTableAddressPointOffsetVal, + const DenseSet &IgnoredBaseTypes, OptimizationRemarkEmitter &ORE) : F(Func), M(M), Symtab(Symtab), SamplePGO(SamplePGO), VirtualCSInfo(VirtualCSInfo), - VTableAddressPointOffsetVal(VTableAddressPointOffsetVal), ORE(ORE) {} + VTableAddressPointOffsetVal(VTableAddressPointOffsetVal), ORE(ORE), + IgnoredBaseTypes(IgnoredBaseTypes) {} IndirectCallPromoter(const IndirectCallPromoter &) = delete; IndirectCallPromoter &operator=(const IndirectCallPromoter &) = delete; @@ -859,7 +864,7 @@ bool IndirectCallPromoter::isProfitableToCompareVTables( LLVM_DEBUG(dbgs() << "\n"); uint64_t CandidateVTableCount = 0; - SmallVector Types; + for (auto &[GUID, Count] : VTableGUIDAndCounts) { CandidateVTableCount += Count; auto *VTableVar = Symtab->getGlobalVariable(GUID); @@ -867,16 +872,14 @@ bool IndirectCallPromoter::isProfitableToCompareVTables( assert(VTableVar && "VTableVar must exist for GUID in VTableGUIDAndCounts"); - Types.clear(); + SmallVector Types; VTableVar->getMetadata(LLVMContext::MD_type, Types); - const DenseSet &VTableSet = ICPIgnoredBaseTypes.getValue(); for (auto *Type : Types) if (auto *TypeId = dyn_cast(Type->getOperand(1).get())) - if (VTableSet.contains(TypeId->getString().str())) { - LLVM_DEBUG(dbgs() - << " vtable profiles are known to be " - "unrepresentative. Bail out vtable comparison."); + if (IgnoredBaseTypes.contains(TypeId->getString())) { + LLVM_DEBUG(dbgs() << " vtable profiles should be ignored. Bail " + "out vtable comparison."); return false; } } @@ -983,9 +986,15 @@ static bool promoteIndirectCalls(Module &M, ProfileSummaryInfo *PSI, bool InLTO, bool Changed = false; VirtualCallSiteTypeInfoMap VirtualCSInfo; - if (EnableVTableProfileUse) + DenseSet IgnoredBaseTypes; + + if (EnableVTableProfileUse) { computeVirtualCallSiteTypeInfoMap(M, MAM, VirtualCSInfo); + for (StringRef Str : ICPIgnoredBaseTypes) + IgnoredBaseTypes.insert(Str); + } + // VTableAddressPointOffsetVal stores the vtable address points. The vtable // address point of a given is static (doesn't // change after being computed once). @@ -1004,7 +1013,8 @@ static bool promoteIndirectCalls(Module &M, ProfileSummaryInfo *PSI, bool InLTO, auto &ORE = FAM.getResult(F); IndirectCallPromoter CallPromoter(F, M, &Symtab, SamplePGO, VirtualCSInfo, - VTableAddressPointOffsetVal, ORE); + VTableAddressPointOffsetVal, + IgnoredBaseTypes, ORE); bool FuncChanged = CallPromoter.processFunction(PSI); if (ICPDUMPAFTER && FuncChanged) { LLVM_DEBUG(dbgs() << "\n== IR Dump After =="; F.print(dbgs())); diff --git a/llvm/test/Transforms/PGOProfile/icp_vtable_cmp.ll b/llvm/test/Transforms/PGOProfile/icp_vtable_cmp.ll index f3a43a830fa12..e9657c914d29e 100644 --- a/llvm/test/Transforms/PGOProfile/icp_vtable_cmp.ll +++ b/llvm/test/Transforms/PGOProfile/icp_vtable_cmp.ll @@ -2,7 +2,7 @@ ; RUN: opt < %s -passes='pgo-icall-prom' -pass-remarks=pgo-icall-prom -enable-vtable-profile-use -icp-max-num-vtable-last-candidate=2 -S 2>&1 | FileCheck %s --check-prefixes=VTABLE-COMMON,VTABLE-CMP ; RUN: opt < %s -passes='pgo-icall-prom' -pass-remarks=pgo-icall-prom -enable-vtable-profile-use -icp-max-num-vtable-last-candidate=1 -S 2>&1 | FileCheck %s --check-prefixes=VTABLE-COMMON,FUNC-CMP -; RUN: opt < %s -passes='pgo-icall-prom' -pass-remarks=pgo-icall-prom -enable-vtable-profile-use -icp-max-num-vtable-last-candidate=1 -icp-ignored-base-types='Base1,Derived3' -S 2>&1 | FileCheck %s --check-prefixes=VTABLE-COMMON,FUNC-CMP +; RUN: opt < %s -passes='pgo-icall-prom' -pass-remarks=pgo-icall-prom -enable-vtable-profile-use -icp-max-num-vtable-last-candidate=1 -icp-ignored-base-types='Base1' -S 2>&1 | FileCheck %s --check-prefixes=VTABLE-COMMON,FUNC-CMP 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" From 255edcebaaa30d70741a9904d900810cccf3bf10 Mon Sep 17 00:00:00 2001 From: mingmingl Date: Tue, 1 Oct 2024 13:06:39 -0700 Subject: [PATCH 4/5] resolve review feedback --- .../lib/Transforms/Instrumentation/IndirectCallPromotion.cpp | 3 +-- llvm/test/Transforms/PGOProfile/icp_vtable_cmp.ll | 5 ++++- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/llvm/lib/Transforms/Instrumentation/IndirectCallPromotion.cpp b/llvm/lib/Transforms/Instrumentation/IndirectCallPromotion.cpp index 83e717b4bcd2e..2ce6edf12246f 100644 --- a/llvm/lib/Transforms/Instrumentation/IndirectCallPromotion.cpp +++ b/llvm/lib/Transforms/Instrumentation/IndirectCallPromotion.cpp @@ -138,8 +138,7 @@ static cl::list ICPIgnoredBaseTypes( "A list of mangled vtable names. Classes specified by the vtables " "and their derived ones will not be vtable-ICP'ed. Useful when the " "profiled types and actual types in the optimized binary could be " - "different due to profiling " - "limitations.")); + "different due to profiling limitations.")); namespace { diff --git a/llvm/test/Transforms/PGOProfile/icp_vtable_cmp.ll b/llvm/test/Transforms/PGOProfile/icp_vtable_cmp.ll index e9657c914d29e..84bb7a5830af2 100644 --- a/llvm/test/Transforms/PGOProfile/icp_vtable_cmp.ll +++ b/llvm/test/Transforms/PGOProfile/icp_vtable_cmp.ll @@ -1,8 +1,11 @@ ; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 5 +; Tests that ICP compares vtables by checking IR. ; RUN: opt < %s -passes='pgo-icall-prom' -pass-remarks=pgo-icall-prom -enable-vtable-profile-use -icp-max-num-vtable-last-candidate=2 -S 2>&1 | FileCheck %s --check-prefixes=VTABLE-COMMON,VTABLE-CMP +; Require exactly one vtable candidate for each function candidate. Tests that ICP compares function by checking IR. ; RUN: opt < %s -passes='pgo-icall-prom' -pass-remarks=pgo-icall-prom -enable-vtable-profile-use -icp-max-num-vtable-last-candidate=1 -S 2>&1 | FileCheck %s --check-prefixes=VTABLE-COMMON,FUNC-CMP -; RUN: opt < %s -passes='pgo-icall-prom' -pass-remarks=pgo-icall-prom -enable-vtable-profile-use -icp-max-num-vtable-last-candidate=1 -icp-ignored-base-types='Base1' -S 2>&1 | FileCheck %s --check-prefixes=VTABLE-COMMON,FUNC-CMP +; On top of line 4, ignore 'Base1' and its derived types for vtable-based comparison. Tests that ICP compares functions. +; RUN: opt < %s -passes='pgo-icall-prom' -pass-remarks=pgo-icall-prom -enable-vtable-profile-use -icp-max-num-vtable-last-candidate=2 -icp-ignored-base-types='Base1' -S 2>&1 | FileCheck %s --check-prefixes=VTABLE-COMMON,FUNC-CMP 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" From 28e236327be7a112d48c2d3e60b61512e4063931 Mon Sep 17 00:00:00 2001 From: mingmingl Date: Tue, 1 Oct 2024 22:25:04 -0700 Subject: [PATCH 5/5] 1. resolve review feedback 2. Update option description to mention the strings are type info names (the string literals in llvm type metadata) --- .../Instrumentation/IndirectCallPromotion.cpp | 50 ++++++++++++------- 1 file changed, 32 insertions(+), 18 deletions(-) diff --git a/llvm/lib/Transforms/Instrumentation/IndirectCallPromotion.cpp b/llvm/lib/Transforms/Instrumentation/IndirectCallPromotion.cpp index 2ce6edf12246f..b605dfc9c2675 100644 --- a/llvm/lib/Transforms/Instrumentation/IndirectCallPromotion.cpp +++ b/llvm/lib/Transforms/Instrumentation/IndirectCallPromotion.cpp @@ -135,10 +135,11 @@ static cl::opt ICPMaxNumVTableLastCandidate( static cl::list ICPIgnoredBaseTypes( "icp-ignored-base-types", cl::Hidden, cl::desc( - "A list of mangled vtable names. Classes specified by the vtables " - "and their derived ones will not be vtable-ICP'ed. Useful when the " - "profiled types and actual types in the optimized binary could be " - "different due to profiling limitations.")); + "A list of mangled vtable type info names. Classes specified by the " + "type info names and their derived ones will not be vtable-ICP'ed. " + "Useful when the profiled types and actual types in the optimized " + "binary could be different due to profiling limitations. Type info " + "names are those string literals used in LLVM type metadata")); namespace { @@ -376,6 +377,10 @@ class IndirectCallPromoter { bool isProfitableToCompareVTables(const CallBase &CB, ArrayRef Candidates); + // Return true if the vtable corresponding to VTableGUID should be skipped + // for vtable-based comparison. + bool shouldSkipVTable(uint64_t VTableGUID); + // Given an indirect callsite and the list of function candidates, compute // the following vtable information in output parameters and return vtable // pointer if type profiles exist. @@ -866,21 +871,9 @@ bool IndirectCallPromoter::isProfitableToCompareVTables( for (auto &[GUID, Count] : VTableGUIDAndCounts) { CandidateVTableCount += Count; - auto *VTableVar = Symtab->getGlobalVariable(GUID); - - assert(VTableVar && - "VTableVar must exist for GUID in VTableGUIDAndCounts"); - - SmallVector Types; - VTableVar->getMetadata(LLVMContext::MD_type, Types); - for (auto *Type : Types) - if (auto *TypeId = dyn_cast(Type->getOperand(1).get())) - if (IgnoredBaseTypes.contains(TypeId->getString())) { - LLVM_DEBUG(dbgs() << " vtable profiles should be ignored. Bail " - "out vtable comparison."); - return false; - } + if (shouldSkipVTable(GUID)) + return false; } if (CandidateVTableCount < Candidate.Count * ICPVTablePercentageThreshold) { @@ -912,6 +905,27 @@ bool IndirectCallPromoter::isProfitableToCompareVTables( return true; } +bool IndirectCallPromoter::shouldSkipVTable(uint64_t VTableGUID) { + if (IgnoredBaseTypes.empty()) + return false; + + auto *VTableVar = Symtab->getGlobalVariable(VTableGUID); + + assert(VTableVar && "VTableVar must exist for GUID in VTableGUIDAndCounts"); + + SmallVector Types; + VTableVar->getMetadata(LLVMContext::MD_type, Types); + + for (auto *Type : Types) + if (auto *TypeId = dyn_cast(Type->getOperand(1).get())) + if (IgnoredBaseTypes.contains(TypeId->getString())) { + LLVM_DEBUG(dbgs() << " vtable profiles should be ignored. Bail " + "out of vtable comparison."); + return true; + } + return false; +} + // For virtual calls in the module, collect per-callsite information which will // be used to associate an ICP candidate with a vtable and a specific function // in the vtable. With type intrinsics (llvm.type.test), we can find virtual