[clang-tools-extra] [compiler-rt] [llvm] [clang] [IRPGO][ValueProfile] Instrument virtual table address that could be used to do virtual table address comparision for indirect-call-promotion. (PR #668
@@ -0,0 +1,117 @@ +To update the inputs used below, run Inputs/update_vtable_value_prof_inputs.sh /path/to/updated/clang++ + +Show profile data from raw profiles. +RUN: llvm-profdata show --function=main --ic-targets --show-vtables %p/Inputs/vtable_prof.profraw | FileCheck %s --check-prefix=RAW + +Generate indexed profile from raw profile and show the data. +RUN: llvm-profdata merge %p/Inputs/vtable_prof.profraw -o %t_indexed.profdata +RUN: llvm-profdata show --function=main --ic-targets --show-vtables %t_indexed.profdata | FileCheck %s --check-prefix=INDEXED + +Generate text profile from raw profile and show the data. +RUN: llvm-profdata merge --text %p/Inputs/vtable_prof.profraw -o %t.proftext +RUN: llvm-profdata show --function=main --ic-targets --show-vtables --text %t.proftext | FileCheck %s --check-prefix=ICTEXT + +RAW: Counters: +RAW-NEXT: main: +RAW-NEXT: Hash: 0x0f9a16fe6d398548 +RAW-NEXT: Counters: 2 +RAW-NEXT: Indirect Call Site Count: 2 +RAW-NEXT: Number of instrumented vtables: 2 +RAW-NEXT: Indirect Target Results: +RAW-NEXT: [ 0, _ZN8Derived15func1Eii,250 ] (25.00%) +RAW-NEXT: [ 0, _ZN8Derived25func1Eii,750 ] (75.00%) +RAW-NEXT: [ 1, _ZN8Derived15func2Eii,250 ] (25.00%) +RAW-NEXT: [ 1, _ZN8Derived25func2Eii,750 ] (75.00%) +RAW-NEXT: VTable Results: +RAW-NEXT: [ 0, _ZTV8Derived1,250 ] (25.00%) +RAW-NEXT:[ 0, _ZTV8Derived2,750 ] (75.00%) +RAW-NEXT:[ 1, _ZTV8Derived1,250 ] (25.00%) +RAW-NEXT: [ 1, _ZTV8Derived2,750 ] (75.00%) +RAW-NEXT: Instrumentation level: IR entry_first = 0 +RAW-NEXT: Functions shown: 1 +RAW-NEXT: Total functions: 6 +RAW-NEXT: Maximum function count: 1000 +RAW-NEXT: Maximum internal block count: 250 +RAW-NEXT: Statistics for indirect call sites profile: +RAW-NEXT: Total number of sites: 2 +RAW-NEXT: Total number of sites with values: 2 +RAW-NEXT: Total number of profiled values: 4 +RAW-NEXT: Value sites histogram: +RAW-NEXT: NumTargets, SiteCount +RAW-NEXT: 2, 2 +RAW-NEXT: Statistics for vtable profile: +RAW-NEXT: Total number of sites: 2 +RAW-NEXT: Total number of sites with values: 2 +RAW-NEXT: Total number of profiled values: 4 +RAW-NEXT: Value sites histogram: +RAW-NEXT:NumTargets, SiteCount +RAW-NEXT: 2, 2 + + +INDEXED: Counters: +INDEXED-NEXT: main: +INDEXED-NEXT: Hash: 0x0f9a16fe6d398548 +INDEXED-NEXT: Counters: 2 +INDEXED-NEXT: Indirect Call Site Count: 2 +INDEXED-NEXT: Number of instrumented vtables: 2 +INDEXED-NEXT: Indirect Target Results: +INDEXED-NEXT: [ 0, _ZN8Derived25func1Eii,750 ] (75.00%) +INDEXED-NEXT: [ 0, _ZN8Derived15func1Eii,250 ] (25.00%) +INDEXED-NEXT: [ 1, _ZN8Derived25func2Eii,750 ] (75.00%) +INDEXED-NEXT: [ 1, _ZN8Derived15func2Eii,250 ] (25.00%) +INDEXED-NEXT: VTable Results: +INDEXED-NEXT:[ 0, _ZTV8Derived2,750 ] (75.00%) +INDEXED-NEXT:[ 0, _ZTV8Derived1,250 ] (25.00%) minglotus-6 wrote: The indentation looks fine with latest few commits now. Going to resolve this. https://github.com/llvm/llvm-project/pull/66825 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [llvm] [compiler-rt] [clang] [IRPGO][ValueProfile] Instrument virtual table address that could be used to do virtual table address comparision for indirect-call-promotion. (PR #668
@@ -3045,10 +3071,10 @@ static int show_main(int argc, const char *argv[]) { if (ProfileKind == instr) return showInstrProfile( Filename, ShowCounts, TopNFunctions, ShowIndirectCallTargets, -ShowMemOPSizes, ShowDetailedSummary, DetailedSummaryCutoffs, -ShowAllFunctions, ShowCS, ValueCutoff, OnlyListBelow, ShowFunction, -TextFormat, ShowBinaryIds, ShowCovered, ShowProfileVersion, -ShowTemporalProfTraces, SFormat, OS); +ShowMemOPSizes, ShowVTables, ShowDetailedSummary, minglotus-6 wrote: I sent out https://github.com/llvm/llvm-project/pull/71328 to do the refactor. Given that the refactor patch would probably be large, and this PR is large itself (touches file that needs to be merged), I'll take the liberty to resolve this in a follow-up patch. https://github.com/llvm/llvm-project/pull/66825 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[llvm] [clang] [clang-tools-extra] [nfc][llvm-profdata] Use cl::Subcommand to organize subcommand and options in llvm-profdata (PR #71328)
https://github.com/minglotus-6 edited https://github.com/llvm/llvm-project/pull/71328 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [llvm] [clang] [nfc][llvm-profdata] Use cl::Subcommand to organize subcommand and options in llvm-profdata (PR #71328)
@@ -47,6 +47,21 @@ using namespace llvm; +// https://llvm.org/docs/CommandGuide/llvm-profdata.html has documentations +// on each subcommand. +cl::SubCommand +ShowSubcommand("show", + "Takes a profile data file and displays the profiles"); +cl::SubCommand +OrderSubcommand("order", +"Reads temporal profiling traces from a " +"profile and outputs a function order that reduces the " +"number of page faults for those traces"); +cl::SubCommand +OverlapSubcommand("overlap", + "Computes and displays the overlap between two profiles"); +cl::SubCommand MergeSubcommand("merge", "Merges profiles"); minglotus-6 wrote: done. https://github.com/llvm/llvm-project/pull/71328 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [llvm] [nfc][llvm-profdata] Use cl::Subcommand to organize subcommand and options in llvm-profdata (PR #71328)
https://github.com/minglotus-6 commented: thanks for the reviews! https://github.com/llvm/llvm-project/pull/71328 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [llvm] [clang] [nfc][llvm-profdata] Use cl::Subcommand to organize subcommand and options in llvm-profdata (PR #71328)
@@ -435,13 +450,195 @@ static void writeInstrProfile(StringRef OutputFilename, } } -static void -mergeInstrProfile(const WeightedFileVector &Inputs, StringRef DebugInfoFilename, - SymbolRemapper *Remapper, StringRef OutputFilename, - ProfileFormat OutputFormat, uint64_t TraceReservoirSize, - uint64_t MaxTraceLength, int MaxDbgCorrelationWarnings, - bool OutputSparse, unsigned NumThreads, FailureMode FailMode, - const StringRef ProfiledBinary) { +// Common options. minglotus-6 wrote: Sounds reasonable to me. I moved options definitions right after `cl::Subcommand`. https://github.com/llvm/llvm-project/pull/71328 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[llvm] [clang] [clang-tools-extra] [nfc][llvm-profdata] Use cl::Subcommand to organize subcommand and options in llvm-profdata (PR #71328)
@@ -1362,28 +1413,55 @@ static int merge_main(int argc, const char *argv[]) { exitWithError( "-supplement-instr-with-sample can only work with -instr. "); -supplementInstrProfile(WeightedInputs, SupplInstrWithSample, OutputFilename, - OutputFormat, OutputSparse, SupplMinSizeThreshold, - ZeroCounterThreshold, InstrProfColdThreshold); +supplementInstrProfile(WeightedInputs, SupplInstrWithSample, OutputSparse, + SupplMinSizeThreshold, ZeroCounterThreshold, + InstrProfColdThreshold); return 0; } if (ProfileKind == instr) -mergeInstrProfile(WeightedInputs, DebugInfoFilename, Remapper.get(), - OutputFilename, OutputFormat, - TemporalProfTraceReservoirSize, - TemporalProfMaxTraceLength, MaxDbgCorrelationWarnings, - OutputSparse, NumThreads, FailureMode, ProfiledBinary); +mergeInstrProfile(WeightedInputs, Remapper.get(), MaxDbgCorrelationWarnings, + ProfiledBinary); else -mergeSampleProfile(WeightedInputs, Remapper.get(), OutputFilename, - OutputFormat, ProfileSymbolListFile, CompressAllSections, - UseMD5, GenPartialProfile, ProfileLayout, - SampleMergeColdContext, SampleTrimColdContext, - SampleColdContextFrameDepth, FailureMode, - DropProfileSymbolList, OutputSizeLimit); +mergeSampleProfile(WeightedInputs, Remapper.get(), ProfileSymbolListFile, + OutputSizeLimit); return 0; } +// Overlap options. +cl::opt BaseFilename(cl::Positional, cl::Required, + cl::desc(""), + cl::sub(OverlapSubcommand)); +cl::opt TestFilename(cl::Positional, cl::Required, + cl::desc(""), + cl::sub(OverlapSubcommand)); + +cl::opt SimilarityCutoff( +"similarity-cutoff", cl::init(0), +cl::desc("For sample profiles, list function names (with calling context " + "for csspgo) for overlapped functions " + "with similarities below the cutoff (percentage times 1)."), +cl::sub(OverlapSubcommand)); + +cl::opt IsCS( +"cs", cl::init(false), +cl::desc("For context sensitive PGO counts. Does not work with CSSPGO."), +cl::sub(OverlapSubcommand)); + +cl::opt FuncNameFilter( minglotus-6 wrote: good catch. I moved it to the 2-common options section. https://github.com/llvm/llvm-project/pull/71328 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [PGO][GlobalValue][LTO]In GlobalValues::getGlobalIdentifier, use semicolon as delimiter for local-linkage varibles. (PR #74008)
https://github.com/minglotus-6 updated https://github.com/llvm/llvm-project/pull/74008 >From 4cb5b087485124a7f2375fdc018b42a0401e6409 Mon Sep 17 00:00:00 2001 From: mingmingl Date: Thu, 30 Nov 2023 15:41:37 -0800 Subject: [PATCH 1/3] [PGO][GlobalValue][LTO]In GlobalValues::getGlobalIdentifier, use semicolon as delimiter for local-linkage varibles. Commit fe05193 (phab D156569), IRPGO names uses format '[;]' while prior format is [:'. The format change would break the use caes demonstrated in (updated) llvm/test/Transforms/PGOProfile/thinlto_indirect_call_promotion.ll This patch changes GlobalValues::getGlobalIdentifer to use the semicolon. To elaborate on the scenario how things break without this PR 1. IRPGO raw profiles stores (compressed) IRPGO names of functions in one section, and per-function profile data in another section. One field in per-function profile data is the MD5 hash of IRPGO names. 2. When raw profiles are converted to indexed format profiles, the profiled address is mapped to the MD5 hash of the callee. 3. In thin-lto prelink pipeline, MD5 hash of IRPGO names will be annotated as value profiles, and used to import indirect-call-prom candidates. If the annotated MD5 hash is computed from the new format while import uses the prior format, the callee cannot be imported. The updated test case Transforms/PGOProfile/thinlto_indirect_call_promotion.ll exercise the following path - Annotate raw profiles and generate import summaries. Using the imported summaries, it tests that functions are correctly imported and ICP transformations happened. --- llvm/lib/IR/Globals.cpp | 4 +- llvm/lib/ProfileData/InstrProf.cpp| 28 +-- .../thinlto-function-summary-originalnames.ll | 6 +- llvm/test/ThinLTO/X86/memprof-basic.ll| 26 +++ .../X86/memprof-duplicate-context-ids.ll | 12 +-- .../ThinLTO/X86/memprof-funcassigncloning.ll | 6 +- llvm/test/ThinLTO/X86/memprof-indirectcall.ll | 32 llvm/test/ThinLTO/X86/memprof-inlined.ll | 14 ++-- .../Inputs/thinlto_icall_prom.profdata| Bin 0 -> 976 bytes .../Inputs/thinlto_indirect_call_promotion.ll | 32 ++-- .../Inputs/update_icall_promotion_inputs.sh | 70 ++ .../thinlto_indirect_call_promotion.ll| 52 ++--- 12 files changed, 194 insertions(+), 88 deletions(-) create mode 100644 llvm/test/Transforms/PGOProfile/Inputs/thinlto_icall_prom.profdata create mode 100644 llvm/test/Transforms/PGOProfile/Inputs/update_icall_promotion_inputs.sh diff --git a/llvm/lib/IR/Globals.cpp b/llvm/lib/IR/Globals.cpp index 7bd4503a689e4..e821de3b198f1 100644 --- a/llvm/lib/IR/Globals.cpp +++ b/llvm/lib/IR/Globals.cpp @@ -158,9 +158,9 @@ std::string GlobalValue::getGlobalIdentifier(StringRef Name, // that it will stay the same, e.g., if the files are checked out from // version control in different locations. if (FileName.empty()) - NewName = NewName.insert(0, ":"); + NewName = NewName.insert(0, ";"); else - NewName = NewName.insert(0, FileName.str() + ":"); + NewName = NewName.insert(0, FileName.str() + ";"); } return NewName; } diff --git a/llvm/lib/ProfileData/InstrProf.cpp b/llvm/lib/ProfileData/InstrProf.cpp index 236b083a1e215..d9ad5c8b6f683 100644 --- a/llvm/lib/ProfileData/InstrProf.cpp +++ b/llvm/lib/ProfileData/InstrProf.cpp @@ -246,11 +246,27 @@ std::string InstrProfError::message() const { char InstrProfError::ID = 0; -std::string getPGOFuncName(StringRef RawFuncName, - GlobalValue::LinkageTypes Linkage, +std::string getPGOFuncName(StringRef Name, GlobalValue::LinkageTypes Linkage, StringRef FileName, uint64_t Version LLVM_ATTRIBUTE_UNUSED) { - return GlobalValue::getGlobalIdentifier(RawFuncName, Linkage, FileName); + // Value names may be prefixed with a binary '1' to indicate + // that the backend should not modify the symbols due to any platform + // naming convention. Do not include that '1' in the PGO profile name. + if (Name[0] == '\1') +Name = Name.substr(1); + + std::string NewName = std::string(Name); + if (llvm::GlobalValue::isLocalLinkage(Linkage)) { +// For local symbols, prepend the main file name to distinguish them. +// Do not include the full path in the file name since there's no guarantee +// that it will stay the same, e.g., if the files are checked out from +// version control in different locations. +if (FileName.empty()) + NewName = NewName.insert(0, ":"); +else + NewName = NewName.insert(0, FileName.str() + ":"); + } + return NewName; } // Strip NumPrefix level of directory name from PathNameStr. If the number of @@ -300,12 +316,8 @@ getIRPGONameForGlobalObject(const GlobalObject &GO, GlobalValue::LinkageTypes Linkage, StringRef FileName) { S
[llvm] [clang] [PGO][GlobalValue][LTO]In GlobalValues::getGlobalIdentifier, use semicolon as delimiter for local-linkage varibles. (PR #74008)
@@ -246,11 +246,27 @@ std::string InstrProfError::message() const { char InstrProfError::ID = 0; -std::string getPGOFuncName(StringRef RawFuncName, - GlobalValue::LinkageTypes Linkage, +std::string getPGOFuncName(StringRef Name, GlobalValue::LinkageTypes Linkage, minglotus-6 wrote: undo the rename. I didn't find a way to configure the Github notifications for real-time comments. Guess I'll need to combine email notifications. https://github.com/llvm/llvm-project/pull/74008 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [PGO][GlobalValue][LTO]In GlobalValues::getGlobalIdentifier, use semicolon as delimiter for local-linkage varibles. (PR #74008)
@@ -300,12 +316,8 @@ getIRPGONameForGlobalObject(const GlobalObject &GO, GlobalValue::LinkageTypes Linkage, StringRef FileName) { SmallString<64> Name; - if (llvm::GlobalValue::isLocalLinkage(Linkage)) { -Name.append(FileName.empty() ? "" : FileName); -Name.append(";"); - } Mangler().getNameWithPrefix(Name, &GO, /*CannotUsePrivateLabel=*/true); minglotus-6 wrote: To avoid subtle issues when linkage-name is different from mangled names,,I'm wondering if it warrants a change to use linkage-names (as opposed to mangled name) in `GlobalValue::getGlobalIdentifier` in this PR. Global identifier is supposed to be hash of unique names, and linkage-name is already unique. https://github.com/llvm/llvm-project/pull/74008 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [PGO][GlobalValue][LTO]In GlobalValues::getGlobalIdentifier, use semicolon as delimiter for local-linkage varibles. (PR #74008)
@@ -300,12 +316,8 @@ getIRPGONameForGlobalObject(const GlobalObject &GO, GlobalValue::LinkageTypes Linkage, StringRef FileName) { SmallString<64> Name; - if (llvm::GlobalValue::isLocalLinkage(Linkage)) { -Name.append(FileName.empty() ? "" : FileName); -Name.append(";"); - } Mangler().getNameWithPrefix(Name, &GO, /*CannotUsePrivateLabel=*/true); minglotus-6 wrote: > I think it makes more sense to use linkage-names for IRPGO, -order_file, and > ThinLTO. -order_file is used in the linker when it only knows linkage-names, > so I don't think it makes sense to feed it mangled names. Thanks for the input. (not to bikeshed but) I think for the purpose of computing global identifier, unique names should suffice. Clang-FE mangled names are unique so sounds fine. Using linkage-name would be a non-trivial change, given the static `getGlobalIdentifier` takes a stringified name currently, and using linkage-name means requiring compatible change in each callsite (e.g., if the caller context doesn't have `GlobalValue` but just stringified names in the bitcode, make sure linkage-name exists in the bitcode, [this](https://github.com/llvm/llvm-project/blob/d6fbd96e5eaf3e8acbf1b43dce7a311352907567/llvm/lib/Bitcode/Reader/BitcodeReader.cpp#L6763) seems one example). An alternative fixup is to store the MD5 of `[filename;]linkage-name` (this is what's currently stored in this [field](https://github.com/llvm/llvm-project/blob/0f45e45847a5f2969b8021c787a566531fc96473/compiler-rt/include/profile/InstrProfData.inc#L72-L74)) and the MD5 of `[filename:]mangled-name` (the original hash before D156569) in the raw profiles, so `llvm-profdata` could choose properly (former for ordering and latter for ICP) Would you mind if I create a Github issue to track how to fix other potential cases and assign it to you ? This PR would solve the colon and semicolon difference. https://github.com/llvm/llvm-project/pull/74008 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [PGO][GlobalValue][LTO]In GlobalValues::getGlobalIdentifier, use semicolon as delimiter for local-linkage varibles. (PR #74008)
https://github.com/minglotus-6 edited https://github.com/llvm/llvm-project/pull/74008 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[llvm] [clang] [PGO][GlobalValue][LTO]In GlobalValues::getGlobalIdentifier, use semicolon as delimiter for local-linkage varibles. (PR #74008)
https://github.com/minglotus-6 edited https://github.com/llvm/llvm-project/pull/74008 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[llvm] [clang] [PGO][GlobalValue][LTO]In GlobalValues::getGlobalIdentifier, use semicolon as delimiter for local-linkage varibles. (PR #74008)
@@ -300,12 +316,8 @@ getIRPGONameForGlobalObject(const GlobalObject &GO, GlobalValue::LinkageTypes Linkage, StringRef FileName) { SmallString<64> Name; - if (llvm::GlobalValue::isLocalLinkage(Linkage)) { -Name.append(FileName.empty() ? "" : FileName); -Name.append(";"); - } Mangler().getNameWithPrefix(Name, &GO, /*CannotUsePrivateLabel=*/true); minglotus-6 wrote: Thanks for giving the thumb-up Ellis! Just created https://github.com/llvm/llvm-project/issues/74565. https://github.com/llvm/llvm-project/pull/74008 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[llvm] [clang] [PGO][GlobalValue][LTO]In GlobalValues::getGlobalIdentifier, use semicolon as delimiter for local-linkage varibles. (PR #74008)
minglotus-6 wrote: Using the same added ICP test, profile matching on local-linkage `_ZL7callee0v` using `clang++ -v -fuse-ld=lld -O2 -fprofile-use=thinlto_icall_prom.profdata ` , as [this](https://gist.github.com/minglotus-6/11817ba645c6b12cd7116f41bfb1185e) pgo-instr-use output shows. However, when trying to follow the counter matching [code path](https://github.com/llvm/llvm-project/blob/df7545e4be5cb65ef3ddd278119c0b8b2f37b0ec/llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp#L1337), I came across another place(https://github.com/llvm/llvm-project/blob/df7545e4be5cb65ef3ddd278119c0b8b2f37b0ec/llvm/lib/ProfileData/InstrProfReader.cpp#L1019) that might have something to do with colon/semicolon delimiter inside itanium remapper (https://github.com/llvm/llvm-project/blob/df7545e4be5cb65ef3ddd278119c0b8b2f37b0ec/llvm/lib/ProfileData/InstrProfReader.cpp#L1055). I'm wondering if this needs an update (with updated regression test)? @david-xl @xur-llvm who might have more context. https://github.com/llvm/llvm-project/pull/74008 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[libc] [compiler-rt] [lldb] [llvm] [lld] [libcxx] [flang] [clang-tools-extra] [libunwind] [mlir] [clang] [IRPGO][ValueProfile] Instrument virtual table address that could be used to do virtual table a
minglotus-6 wrote: Three tests failed on the windows build-bot (https://buildkite.com/llvm-project/github-pull-requests/builds/20392#018c3c8d-d4fd-41f7-b456-b1a4ded6dc61). Two (Transforms/PGOProfile/vtable_profile.ll and tools/llvm-profdata/vtable-value-prof-basic.test) of these three passed in my local check and one (tools/llvm-profdata/nocompress.test) turns out not supported (`requires !zlib`) in my local build I added `require zlib` for the first two, and updated the 3rd test under `-DLLVM_ENABLE_ZLIB=0`. https://github.com/llvm/llvm-project/pull/66825 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[llvm] [clang] [PGO][GlobalValue][LTO]In GlobalValues::getGlobalIdentifier, use semicolon as delimiter for local-linkage varibles. (PR #74008)
minglotus-6 wrote: > Can you clarify what you are saying here - is it working or not working? It's working. The local-linkage `_ZL7callee0v` has function entry count annotated as `!prof`. David says the itanium remapper file was only used once during gcc to llvm transition, so not relevant here. https://github.com/llvm/llvm-project/pull/74008 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [PGO][GlobalValue][LTO]In GlobalValues::getGlobalIdentifier, use semicolon as delimiter for local-linkage varibles. (PR #74008)
minglotus-6 wrote: Just noticed there is a merge conflict. Will update my fork and merge. https://github.com/llvm/llvm-project/pull/74008 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[llvm] [clang-tools-extra] [compiler-rt] [clang] [PGO][GlobalValue][LTO]In GlobalValues::getGlobalIdentifier, use semicolon as delimiter for local-linkage varibles. (PR #74008)
https://github.com/minglotus-6 updated https://github.com/llvm/llvm-project/pull/74008 >From 4cb5b087485124a7f2375fdc018b42a0401e6409 Mon Sep 17 00:00:00 2001 From: mingmingl Date: Thu, 30 Nov 2023 15:41:37 -0800 Subject: [PATCH 1/3] [PGO][GlobalValue][LTO]In GlobalValues::getGlobalIdentifier, use semicolon as delimiter for local-linkage varibles. Commit fe05193 (phab D156569), IRPGO names uses format '[;]' while prior format is [:'. The format change would break the use caes demonstrated in (updated) llvm/test/Transforms/PGOProfile/thinlto_indirect_call_promotion.ll This patch changes GlobalValues::getGlobalIdentifer to use the semicolon. To elaborate on the scenario how things break without this PR 1. IRPGO raw profiles stores (compressed) IRPGO names of functions in one section, and per-function profile data in another section. One field in per-function profile data is the MD5 hash of IRPGO names. 2. When raw profiles are converted to indexed format profiles, the profiled address is mapped to the MD5 hash of the callee. 3. In thin-lto prelink pipeline, MD5 hash of IRPGO names will be annotated as value profiles, and used to import indirect-call-prom candidates. If the annotated MD5 hash is computed from the new format while import uses the prior format, the callee cannot be imported. The updated test case Transforms/PGOProfile/thinlto_indirect_call_promotion.ll exercise the following path - Annotate raw profiles and generate import summaries. Using the imported summaries, it tests that functions are correctly imported and ICP transformations happened. --- llvm/lib/IR/Globals.cpp | 4 +- llvm/lib/ProfileData/InstrProf.cpp| 28 +-- .../thinlto-function-summary-originalnames.ll | 6 +- llvm/test/ThinLTO/X86/memprof-basic.ll| 26 +++ .../X86/memprof-duplicate-context-ids.ll | 12 +-- .../ThinLTO/X86/memprof-funcassigncloning.ll | 6 +- llvm/test/ThinLTO/X86/memprof-indirectcall.ll | 32 llvm/test/ThinLTO/X86/memprof-inlined.ll | 14 ++-- .../Inputs/thinlto_icall_prom.profdata| Bin 0 -> 976 bytes .../Inputs/thinlto_indirect_call_promotion.ll | 32 ++-- .../Inputs/update_icall_promotion_inputs.sh | 70 ++ .../thinlto_indirect_call_promotion.ll| 52 ++--- 12 files changed, 194 insertions(+), 88 deletions(-) create mode 100644 llvm/test/Transforms/PGOProfile/Inputs/thinlto_icall_prom.profdata create mode 100644 llvm/test/Transforms/PGOProfile/Inputs/update_icall_promotion_inputs.sh diff --git a/llvm/lib/IR/Globals.cpp b/llvm/lib/IR/Globals.cpp index 7bd4503a689e4..e821de3b198f1 100644 --- a/llvm/lib/IR/Globals.cpp +++ b/llvm/lib/IR/Globals.cpp @@ -158,9 +158,9 @@ std::string GlobalValue::getGlobalIdentifier(StringRef Name, // that it will stay the same, e.g., if the files are checked out from // version control in different locations. if (FileName.empty()) - NewName = NewName.insert(0, ":"); + NewName = NewName.insert(0, ";"); else - NewName = NewName.insert(0, FileName.str() + ":"); + NewName = NewName.insert(0, FileName.str() + ";"); } return NewName; } diff --git a/llvm/lib/ProfileData/InstrProf.cpp b/llvm/lib/ProfileData/InstrProf.cpp index 236b083a1e215..d9ad5c8b6f683 100644 --- a/llvm/lib/ProfileData/InstrProf.cpp +++ b/llvm/lib/ProfileData/InstrProf.cpp @@ -246,11 +246,27 @@ std::string InstrProfError::message() const { char InstrProfError::ID = 0; -std::string getPGOFuncName(StringRef RawFuncName, - GlobalValue::LinkageTypes Linkage, +std::string getPGOFuncName(StringRef Name, GlobalValue::LinkageTypes Linkage, StringRef FileName, uint64_t Version LLVM_ATTRIBUTE_UNUSED) { - return GlobalValue::getGlobalIdentifier(RawFuncName, Linkage, FileName); + // Value names may be prefixed with a binary '1' to indicate + // that the backend should not modify the symbols due to any platform + // naming convention. Do not include that '1' in the PGO profile name. + if (Name[0] == '\1') +Name = Name.substr(1); + + std::string NewName = std::string(Name); + if (llvm::GlobalValue::isLocalLinkage(Linkage)) { +// For local symbols, prepend the main file name to distinguish them. +// Do not include the full path in the file name since there's no guarantee +// that it will stay the same, e.g., if the files are checked out from +// version control in different locations. +if (FileName.empty()) + NewName = NewName.insert(0, ":"); +else + NewName = NewName.insert(0, FileName.str() + ":"); + } + return NewName; } // Strip NumPrefix level of directory name from PathNameStr. If the number of @@ -300,12 +316,8 @@ getIRPGONameForGlobalObject(const GlobalObject &GO, GlobalValue::LinkageTypes Linkage, StringRef FileName) { S
[clang-tools-extra] [clang] [compiler-rt] [llvm] [PGO][GlobalValue][LTO]In GlobalValues::getGlobalIdentifier, use semicolon as delimiter for local-linkage varibles. (PR #74008)
minglotus-6 wrote: > > > David says the itanium remapper file was only used once during gcc to > > > llvm transition, so not relevant here. > > > > > > I believe it was actually for the libstdc++ to libc++ transition (see > > https://reviews.llvm.org/D51247 and https://reviews.llvm.org/D51240). > > If it is broken we'll at least want to add a FIXME there. > > Yes, I meant libstdc++ to libc++ transition. Why source line is this comment > addressing? I take take a look the changes/comments there. Sorry for the misinformation. I think the itanium remapper needs a `:` -> `;` update (going to update this PR and tests to do it), since (for local-linkage functions) the function name used to look up profiles should use `;` delimiter. https://github.com/llvm/llvm-project/pull/74008 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[compiler-rt] [clang] [llvm] [clang-tools-extra] [PGO][GlobalValue][LTO]In GlobalValues::getGlobalIdentifier, use semicolon as delimiter for local-linkage varibles. (PR #74008)
https://github.com/minglotus-6 updated https://github.com/llvm/llvm-project/pull/74008 >From 4cb5b087485124a7f2375fdc018b42a0401e6409 Mon Sep 17 00:00:00 2001 From: mingmingl Date: Thu, 30 Nov 2023 15:41:37 -0800 Subject: [PATCH 1/5] [PGO][GlobalValue][LTO]In GlobalValues::getGlobalIdentifier, use semicolon as delimiter for local-linkage varibles. Commit fe05193 (phab D156569), IRPGO names uses format '[;]' while prior format is [:'. The format change would break the use caes demonstrated in (updated) llvm/test/Transforms/PGOProfile/thinlto_indirect_call_promotion.ll This patch changes GlobalValues::getGlobalIdentifer to use the semicolon. To elaborate on the scenario how things break without this PR 1. IRPGO raw profiles stores (compressed) IRPGO names of functions in one section, and per-function profile data in another section. One field in per-function profile data is the MD5 hash of IRPGO names. 2. When raw profiles are converted to indexed format profiles, the profiled address is mapped to the MD5 hash of the callee. 3. In thin-lto prelink pipeline, MD5 hash of IRPGO names will be annotated as value profiles, and used to import indirect-call-prom candidates. If the annotated MD5 hash is computed from the new format while import uses the prior format, the callee cannot be imported. The updated test case Transforms/PGOProfile/thinlto_indirect_call_promotion.ll exercise the following path - Annotate raw profiles and generate import summaries. Using the imported summaries, it tests that functions are correctly imported and ICP transformations happened. --- llvm/lib/IR/Globals.cpp | 4 +- llvm/lib/ProfileData/InstrProf.cpp| 28 +-- .../thinlto-function-summary-originalnames.ll | 6 +- llvm/test/ThinLTO/X86/memprof-basic.ll| 26 +++ .../X86/memprof-duplicate-context-ids.ll | 12 +-- .../ThinLTO/X86/memprof-funcassigncloning.ll | 6 +- llvm/test/ThinLTO/X86/memprof-indirectcall.ll | 32 llvm/test/ThinLTO/X86/memprof-inlined.ll | 14 ++-- .../Inputs/thinlto_icall_prom.profdata| Bin 0 -> 976 bytes .../Inputs/thinlto_indirect_call_promotion.ll | 32 ++-- .../Inputs/update_icall_promotion_inputs.sh | 70 ++ .../thinlto_indirect_call_promotion.ll| 52 ++--- 12 files changed, 194 insertions(+), 88 deletions(-) create mode 100644 llvm/test/Transforms/PGOProfile/Inputs/thinlto_icall_prom.profdata create mode 100644 llvm/test/Transforms/PGOProfile/Inputs/update_icall_promotion_inputs.sh diff --git a/llvm/lib/IR/Globals.cpp b/llvm/lib/IR/Globals.cpp index 7bd4503a689e4..e821de3b198f1 100644 --- a/llvm/lib/IR/Globals.cpp +++ b/llvm/lib/IR/Globals.cpp @@ -158,9 +158,9 @@ std::string GlobalValue::getGlobalIdentifier(StringRef Name, // that it will stay the same, e.g., if the files are checked out from // version control in different locations. if (FileName.empty()) - NewName = NewName.insert(0, ":"); + NewName = NewName.insert(0, ";"); else - NewName = NewName.insert(0, FileName.str() + ":"); + NewName = NewName.insert(0, FileName.str() + ";"); } return NewName; } diff --git a/llvm/lib/ProfileData/InstrProf.cpp b/llvm/lib/ProfileData/InstrProf.cpp index 236b083a1e215..d9ad5c8b6f683 100644 --- a/llvm/lib/ProfileData/InstrProf.cpp +++ b/llvm/lib/ProfileData/InstrProf.cpp @@ -246,11 +246,27 @@ std::string InstrProfError::message() const { char InstrProfError::ID = 0; -std::string getPGOFuncName(StringRef RawFuncName, - GlobalValue::LinkageTypes Linkage, +std::string getPGOFuncName(StringRef Name, GlobalValue::LinkageTypes Linkage, StringRef FileName, uint64_t Version LLVM_ATTRIBUTE_UNUSED) { - return GlobalValue::getGlobalIdentifier(RawFuncName, Linkage, FileName); + // Value names may be prefixed with a binary '1' to indicate + // that the backend should not modify the symbols due to any platform + // naming convention. Do not include that '1' in the PGO profile name. + if (Name[0] == '\1') +Name = Name.substr(1); + + std::string NewName = std::string(Name); + if (llvm::GlobalValue::isLocalLinkage(Linkage)) { +// For local symbols, prepend the main file name to distinguish them. +// Do not include the full path in the file name since there's no guarantee +// that it will stay the same, e.g., if the files are checked out from +// version control in different locations. +if (FileName.empty()) + NewName = NewName.insert(0, ":"); +else + NewName = NewName.insert(0, FileName.str() + ":"); + } + return NewName; } // Strip NumPrefix level of directory name from PathNameStr. If the number of @@ -300,12 +316,8 @@ getIRPGONameForGlobalObject(const GlobalObject &GO, GlobalValue::LinkageTypes Linkage, StringRef FileName) { S
[clang] [llvm] [clang-tools-extra] [compiler-rt] [PGO][GlobalValue][LTO]In GlobalValues::getGlobalIdentifier, use semicolon as delimiter for local-linkage varibles. (PR #74008)
minglotus-6 wrote: > > > > > David says the itanium remapper file was only used once during gcc to > > > > > llvm transition, so not relevant here. > > > > > > > > > > > > I believe it was actually for the libstdc++ to libc++ transition (see > > > > https://reviews.llvm.org/D51247 and https://reviews.llvm.org/D51240). > > > > If it is broken we'll at least want to add a FIXME there. > > > > > > > > > Yes, I meant libstdc++ to libc++ transition. Why source line is this > > > comment addressing? I take take a look the changes/comments there. > > > > > > Sorry for the misinformation, and thanks for the Phab links. > > I think the itanium remapper needs a `:` -> `;` update (going to update > > this PR and related tests), since (for local-linkage functions) the > > function name used to look up profiles should use `;` delimiter. > > > > > > David says the itanium remapper file was only used once during gcc to > > > > > llvm transition, so not relevant here. > > > > > > > > > > > > I believe it was actually for the libstdc++ to libc++ transition (see > > > > https://reviews.llvm.org/D51247 and https://reviews.llvm.org/D51240). > > > > If it is broken we'll at least want to add a FIXME there. > > > > > > > > > Yes, I meant libstdc++ to libc++ transition. Why source line is this > > > comment addressing? I take take a look the changes/comments there. > > > > > > Sorry for the misinformation, and thanks for the Phab links. > > I think the itanium remapper needs a `:` -> `;` update (going to update > > this PR and related tests), since (for local-linkage functions) the > > function name used to look up profiles should use `;` delimiter. > > The remapper is not aware of any internal symbol mangling scheme, so those > entires won't be tracked by it. In other words, there is no need to change > anything there, I think. Not updating Itanium remapper should work for PGO counter matching until the next transition (details below); for consistency I just updated Itanium remapper's `extractName` to use semicolon. The details * For PGO counter matching, instr prof reader [asks](https://github.com/llvm/llvm-project/blob/32ec5fbfed32f37aa070ee38e9b038bd84ca6479/llvm/lib/ProfileData/InstrProfReader.cpp#L1339) read remapper for a record. * Only with a remapping file provided (specified by `-fprofile-remapping-file`), a itanium remap reader is constructed. And when remapping file is not specified, a no-op remap reader is constructed. [source code](https://github.com/llvm/llvm-project/blob/32ec5fbfed32f37aa070ee38e9b038bd84ca6479/llvm/lib/ProfileData/InstrProfReader.cpp#L1306-L1314) * When remapping file is specified, remap reader tries to extract the mangled name (removing `filename` prefix`) by finding a `:` (no longer used as delimiter for newer profiles) and remaps the mangled name. If `:` is not updated to `;`, the name is remapped to itself (irpgo func format) and profiles could still be found. However, not updating means remapping becomes no-op for local-linkage functions, which is fine after the transition complete but doesn't work for new transitions (if it happens..) https://github.com/llvm/llvm-project/pull/74008 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[llvm] [clang] [compiler-rt] [clang-tools-extra] [PGO][GlobalValue][LTO]In GlobalValues::getGlobalIdentifier, use semicolon as delimiter for local-linkage varibles. (PR #74008)
minglotus-6 wrote: done. Sorry about leaving debugging-only stuff in a commit. https://github.com/llvm/llvm-project/pull/74008 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [compiler-rt] [llvm] [PGO][GlobalValue][LTO]In GlobalValues::getGlobalIdentifier, use semicolon as delimiter for local-linkage varibles. (PR #74008)
@@ -1,39 +1,45 @@ -; Do setup work for all below tests: generate bitcode and combined index -; RUN: opt -module-summary %s -o %t.bc -; RUN: opt -module-summary %p/Inputs/thinlto_indirect_call_promotion.ll -o %t2.bc +; The raw profiles and reduced IR inputs are generated from Inputs/update_icall_promotion_inputs.sh minglotus-6 wrote: Using a `.proftext` file (and convert it to indexed profiles in the test using `RUN` line) generally has better readability. For better test coverage, I changed to store raw profile data in `Inputs/` directory and added some comments at the beginning of test case Transforms/PGOProfile/thinlto_indirect_call_promotion.ll https://github.com/llvm/llvm-project/pull/74008 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [compiler-rt] [llvm] [PGO][GlobalValue][LTO]In GlobalValues::getGlobalIdentifier, use semicolon as delimiter for local-linkage varibles. (PR #74008)
@@ -1,39 +1,45 @@ -; Do setup work for all below tests: generate bitcode and combined index -; RUN: opt -module-summary %s -o %t.bc -; RUN: opt -module-summary %p/Inputs/thinlto_indirect_call_promotion.ll -o %t2.bc +; The raw profiles and reduced IR inputs are generated from Inputs/update_icall_promotion_inputs.sh + +; Do setup work for all below tests: annotate value profiles, generate bitcode and combined index. +; Explicitly turn off ICP pass in Inputs/thinlto_indirect_call_promotion.ll. +; This way ICP happens in %t.bc after _Z11global_funcv and two indirect callees are imported. +; RUN: opt -passes=pgo-instr-use -pgo-test-profile-file=%p/Inputs/thinlto_icall_prom.profdata -module-summary %s -o %t.bc minglotus-6 wrote: Thanks for the pointer to split-file. Applied it for reduced IR. Choose to use raw profiles (see the other comment) per discussion. https://github.com/llvm/llvm-project/pull/74008 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[compiler-rt] [clang-tools-extra] [clang] [llvm] [PGO][GlobalValue][LTO]In GlobalValues::getGlobalIdentifier, use semicolon as delimiter for local-linkage varibles. (PR #74008)
minglotus-6 wrote: > . For IR PGO, there is basically no need to do so as the instrumentation and > profile-use should be in-sync. For front-end instrumentation, there seem to > be some use cases to use out of sync profile: https://reviews.llvm.org/D51240. Thanks for double checking. I noticed the ICP and stale profile tolerance discussions when read the Phab history; it's good Phab review history are still available nowadays. IRPGO profiles could be used along with supplementary sample-pgo profiles. I'll probably read relevant code in llvm-profdata to understand how these interact in theory mostly for my own curiosity (hopefully no rough edges as long as `llvm-profdata` uses the same pgo name format used by latest compiler) https://github.com/llvm/llvm-project/pull/74008 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[llvm] [compiler-rt] [clang-tools-extra] [clang] [PGO][GlobalValue][LTO]In GlobalValues::getGlobalIdentifier, use semicolon as delimiter for local-linkage varibles. (PR #74008)
@@ -1,39 +1,45 @@ -; Do setup work for all below tests: generate bitcode and combined index -; RUN: opt -module-summary %s -o %t.bc -; RUN: opt -module-summary %p/Inputs/thinlto_indirect_call_promotion.ll -o %t2.bc +; The raw profiles and reduced IR inputs are generated from Inputs/update_icall_promotion_inputs.sh minglotus-6 wrote: The script and c++ source is provided for on-demand update (e.g., version bump, etc). IIUC the goal of a `compiler-rt` test is to generate raw profiles from C++ source for each run of the IR test (`Transforms/PGOProfile/thinlto_indirect_call_promotion.ll`) (i.e.,the IR test would use the the output from the `compiler-rt` test). Practically, is there a way to run two tests one after the other so one could make use of the output of the other? Or shall the `compiler-rt` test verify the content of (raw or indexed) profile content? (I'm assuming the IR test itself stays in `llvm/test/Transforms/PGOProfile` directory makes more sense here) https://github.com/llvm/llvm-project/pull/74008 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [compiler-rt] [clang-tools-extra] [llvm] [PGO][GlobalValue][LTO]In GlobalValues::getGlobalIdentifier, use semicolon as delimiter for local-linkage varibles. (PR #74008)
@@ -1,39 +1,45 @@ -; Do setup work for all below tests: generate bitcode and combined index -; RUN: opt -module-summary %s -o %t.bc -; RUN: opt -module-summary %p/Inputs/thinlto_indirect_call_promotion.ll -o %t2.bc +; The raw profiles and reduced IR inputs are generated from Inputs/update_icall_promotion_inputs.sh minglotus-6 wrote: I'm currently making attempt to have a single test under compiler-rt/test (i.e., removing thinlto_indirect_call_promotion.ll from IR test directory if the attempt works out). Will update back. https://github.com/llvm/llvm-project/pull/74008 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [llvm] [compiler-rt] [clang] [PGO][GlobalValue][LTO]In GlobalValues::getGlobalIdentifier, use semicolon as delimiter for local-linkage varibles. (PR #74008)
minglotus-6 wrote: > > . For IR PGO, there is basically no need to do so as the instrumentation > > and profile-use should be in-sync. For front-end instrumentation, there > > seem to be some use cases to use out of sync profile: > > https://reviews.llvm.org/D51240. > > Thanks for double checking. I noticed the ICP and stale profile tolerance > discussions when read the Phab history; it's good Phab review history are > still available nowadays. > > IRPGO profiles could be used along with supplementary sample-pgo profiles. > I'll probably read relevant code in llvm-profdata to understand how these > interact in theory mostly for my own curiosity (hopefully no rough edges as > long as `llvm-profdata` uses the same pgo name format used by latest compiler) For irpgo with supplementary profiles, this line to build a map (https://github.com/llvm/llvm-project/blob/44dc1e0baae7c4b8a02ba06dcf396d3d452aa873/llvm/tools/llvm-profdata/llvm-profdata.cpp#L982) needs an update. Will do it together with the test [update](https://github.com/llvm/llvm-project/pull/74008#discussion_r1421018997) in this pull request. https://github.com/llvm/llvm-project/pull/74008 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[compiler-rt] [clang-tools-extra] [llvm] [clang] [PGO][GlobalValue][LTO]In GlobalValues::getGlobalIdentifier, use semicolon as delimiter for local-linkage varibles. (PR #74008)
minglotus-6 wrote: > Mach-O (and 32-bit Windows) mangle global symbols with a leading `_`. It's very helpful to know the name differences exist on Mach-O/COFF to construct a test case for issue 74565. https://godbolt.org/z/686Y9vYod shows the name difference (cross comparing IR and machine assembly, with and without `--target arm64-apple-macosx`), meaning the compiler-rt test (to be added in this pull-request) could be a test case for issue 74565. The summary captures key points of the bug. Just to clarify on one detail, the md5 hash is actually computed (or more accurately [mapped](https://github.com/llvm/llvm-project/blob/fc715e4cd942612a091097339841733757b53824/llvm/lib/ProfileData/InstrProf.cpp#L876-L885) from a profiled address to the [NameRef field](https://github.com/llvm/llvm-project/blob/fc715e4cd942612a091097339841733757b53824/compiler-rt/include/profile/InstrProfData.inc#L72) of during `llvm-profdata merge -o `) and [annotated](https://github.com/llvm/llvm-project/blob/fc715e4cd942612a091097339841733757b53824/llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp#L1707) on an indirect-call instruction by `pgo-instr-use` pass. Module-summary-analysis pass makes use of this hash by [extracting](https://github.com/llvm/llvm-project/blob/fc715e4cd942612a091097339841733757b53824/llvm/lib/Analysis/ModuleSummaryAnalysis.cpp#L454-L456) it from value profile metadata. https://github.com/llvm/llvm-project/pull/74008 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [llvm] [clang] [compiler-rt] [PGO][GlobalValue][LTO]In GlobalValues::getGlobalIdentifier, use semicolon as delimiter for local-linkage varibles. (PR #74008)
@@ -144,25 +145,27 @@ void GlobalObject::copyAttributesFrom(const GlobalObject *Src) { std::string GlobalValue::getGlobalIdentifier(StringRef Name, GlobalValue::LinkageTypes Linkage, StringRef FileName) { - // Value names may be prefixed with a binary '1' to indicate // that the backend should not modify the symbols due to any platform // naming convention. Do not include that '1' in the PGO profile name. if (Name[0] == '\1') Name = Name.substr(1); - std::string NewName = std::string(Name); + SmallString<64> GlobalName; if (llvm::GlobalValue::isLocalLinkage(Linkage)) { // For local symbols, prepend the main file name to distinguish them. // Do not include the full path in the file name since there's no guarantee // that it will stay the same, e.g., if the files are checked out from // version control in different locations. if (FileName.empty()) - NewName = NewName.insert(0, ":"); + GlobalName.append(""); else - NewName = NewName.insert(0, FileName.str() + ":"); + GlobalName.append(FileName.str()); + +GlobalName.append({kGlobalIdentifierDelimiter}); minglotus-6 wrote: Done, use `+=` for all previous `append` calls since the operator is [overloaded](https://github.com/llvm/llvm-project/blob/a52ac7f93a31c664d8943242bdafd719d38f2ffa/llvm/include/llvm/ADT/SmallString.h#L279-L286) for `StringRef` and `char` https://github.com/llvm/llvm-project/pull/74008 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[compiler-rt] [clang-tools-extra] [llvm] [clang] [PGO][GlobalValue][LTO]In GlobalValues::getGlobalIdentifier, use semicolon as delimiter for local-linkage varibles. (PR #74008)
@@ -0,0 +1,60 @@ +#!/bin/bash + +if [ $# -lt 1 ]; then + echo "Path to clang required!" + echo "Usage: update_thinlto_indirect_call_promotion_inputs.sh /path/to/updated/clang" + exit 1 +else + CLANG=$1 +fi + +# Allows the script to be invoked from other directories. +OUTDIR=$(dirname $(realpath -s $0)) minglotus-6 wrote: Using `trap` on EXIT is indeed a more reliable way to clean up, thanks for pointing this out! Now the test uses `rm -rf %t && split-file %s %t` as suggested in another comment. https://github.com/llvm/llvm-project/pull/74008 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang] [llvm] [compiler-rt] [PGO][GlobalValue][LTO]In GlobalValues::getGlobalIdentifier, use semicolon as delimiter for local-linkage varibles. (PR #74008)
https://github.com/minglotus-6 edited https://github.com/llvm/llvm-project/pull/74008 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang] [llvm] [compiler-rt] [PGO][GlobalValue][LTO]In GlobalValues::getGlobalIdentifier, use semicolon as delimiter for local-linkage varibles. (PR #74008)
https://github.com/minglotus-6 edited https://github.com/llvm/llvm-project/pull/74008 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [compiler-rt] [llvm] [PGO][GlobalValue][LTO]In GlobalValues::getGlobalIdentifier, use semicolon as delimiter for local-linkage varibles. (PR #74008)
@@ -0,0 +1,97 @@ +// This is a regression test for ThinLTO indirect-call-promotion when candidate +// callees need to be imported from another IR module. In the C++ test case, +// `main` calls `global_func` which is defined in another module. `global_func` +// has two indirect callees, one has external linkage and one has local linkage. +// All three functions should be imported into the IR module of main. + +// What the test does: +// - Generate raw profiles from executables and convert it to indexed profiles. +// During the conversion, a profiled callee address in raw profiles will be +// converted to function hash in indexed profiles. +// - Run IRPGO profile use and ThinTLO prelink pipeline and get LLVM bitcodes +// for both cpp files in the C++ test case. +// - Generate ThinLTO summary file with LLVM bitcodes, and run `function-import` pass. +// - Run `pgo-icall-prom` pass for the IR module which needs to import callees. + +// RUN: rm -rf %t && split-file %s %t && cd %t + +// Use clang*_{pgogen,pgouse} for IR level instrumentation, and use clangxx* for +// C++. +// +// Do setup work for all below tests. +// Generate raw profiles from real programs and convert it into indexed profiles. +// RUN: %clangxx_pgogen -fuse-ld=lld -O2 lib.cpp main.cpp -o main +// RUN: env LLVM_PROFILE_FILE=main.profraw %run ./main +// RUN: llvm-profdata merge main.profraw -o main.profdata + +// Use profile on lib and get bitcode, test that local function callee0 has +// expected !PGOFuncName metadata and external function callee1 doesn't have +// !PGOFuncName metadata. Explicitly skip ICP pass to test ICP happens as +// expected in the IR module that imports functions from lib. +// RUN: %clang -target x86_64-unknown-linux-gnu -mllvm -disable-icp -fprofile-use=main.profdata -flto=thin -O2 -c lib.cpp -o lib.bc minglotus-6 wrote: With this comment, I realized this test should fail on systems where `linkage-name` and `mangled-name` diverges. So added `XFAIL` for darwin systems and `UNSUPPORTED` for win-32 with some comments. https://github.com/llvm/llvm-project/pull/74008 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang] [llvm] [compiler-rt] [PGO][GlobalValue][LTO]In GlobalValues::getGlobalIdentifier, use semicolon as delimiter for local-linkage varibles. (PR #74008)
@@ -144,25 +145,27 @@ void GlobalObject::copyAttributesFrom(const GlobalObject *Src) { std::string GlobalValue::getGlobalIdentifier(StringRef Name, GlobalValue::LinkageTypes Linkage, StringRef FileName) { - // Value names may be prefixed with a binary '1' to indicate // that the backend should not modify the symbols due to any platform // naming convention. Do not include that '1' in the PGO profile name. if (Name[0] == '\1') Name = Name.substr(1); - std::string NewName = std::string(Name); + SmallString<64> GlobalName; minglotus-6 wrote: done. https://github.com/llvm/llvm-project/pull/74008 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang] [llvm] [compiler-rt] [PGO][GlobalValue][LTO]In GlobalValues::getGlobalIdentifier, use semicolon as delimiter for local-linkage varibles. (PR #74008)
@@ -0,0 +1,97 @@ +// This is a regression test for ThinLTO indirect-call-promotion when candidate +// callees need to be imported from another IR module. In the C++ test case, +// `main` calls `global_func` which is defined in another module. `global_func` +// has two indirect callees, one has external linkage and one has local linkage. +// All three functions should be imported into the IR module of main. + +// What the test does: +// - Generate raw profiles from executables and convert it to indexed profiles. +// During the conversion, a profiled callee address in raw profiles will be +// converted to function hash in indexed profiles. +// - Run IRPGO profile use and ThinTLO prelink pipeline and get LLVM bitcodes +// for both cpp files in the C++ test case. +// - Generate ThinLTO summary file with LLVM bitcodes, and run `function-import` pass. +// - Run `pgo-icall-prom` pass for the IR module which needs to import callees. + +// RUN: rm -rf %t && split-file %s %t && cd %t + +// Use clang*_{pgogen,pgouse} for IR level instrumentation, and use clangxx* for +// C++. +// +// Do setup work for all below tests. +// Generate raw profiles from real programs and convert it into indexed profiles. +// RUN: %clangxx_pgogen -fuse-ld=lld -O2 lib.cpp main.cpp -o main +// RUN: env LLVM_PROFILE_FILE=main.profraw %run ./main +// RUN: llvm-profdata merge main.profraw -o main.profdata + +// Use profile on lib and get bitcode, test that local function callee0 has +// expected !PGOFuncName metadata and external function callee1 doesn't have +// !PGOFuncName metadata. Explicitly skip ICP pass to test ICP happens as +// expected in the IR module that imports functions from lib. +// RUN: %clang -target x86_64-unknown-linux-gnu -mllvm -disable-icp -fprofile-use=main.profdata -flto=thin -O2 -c lib.cpp -o lib.bc +// RUN: llvm-dis lib.bc -o - | FileCheck %s --check-prefix=PGOName + +// Use profile on main and get bitcode. +// RUN: %clang -target x86_64-unknown-linux-gnu -fprofile-use=main.profdata -flto=thin -O2 -c main.cpp -o main.bc + +// Run llvm-lto to get summary file. +// RUN: llvm-lto -thinlto -o summary main.bc lib.bc + +// Test the imports of functions. Default import thresholds would work but do +// explicit override to be more futureproof. Note all functions have one basic +// block with a function-entry-count of one, so they are actually hot functions +// per default profile summary hotness cutoff. +// RUN: opt -passes=function-import -import-instr-limit=100 -import-cold-multiplier=1 -summary-file summary.thinlto.bc main.bc -o main.import.bc -print-imports 2>&1 | FileCheck %s --check-prefix=IMPORTS +// Test that '_Z11global_funcv' has indirect calls annotated with value profiles. +// RUN: llvm-dis main.import.bc -o - | FileCheck %s --check-prefix=IR + +// Test that both candidates are ICP'ed and there is no `!VP` in the IR. +// RUN: opt main.import.bc -icp-lto -passes=pgo-icall-prom -S -pass-remarks=pgo-icall-prom 2>&1 | FileCheck %s --check-prefixes=ICP-IR,ICP-REMARK --implicit-check-not="!VP" + +// IMPORTS: main.cpp: Import _Z7callee1v +// IMPORTS: main.cpp: Import _ZL7callee0v.llvm.{{[0-9]+}} minglotus-6 wrote: done. Also used the numerical matcher in other places. https://github.com/llvm/llvm-project/pull/74008 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [compiler-rt] [clang-tools-extra] [llvm] [PGO][GlobalValue][LTO]In GlobalValues::getGlobalIdentifier, use semicolon as delimiter for local-linkage varibles. (PR #74008)
@@ -41,6 +41,10 @@ namespace Intrinsic { typedef unsigned ID; } // end namespace Intrinsic +// Choose ';' as the delimiter. ':' was used once but it doesn't work well for +// Objective-C functions which commonly have :'s in their names minglotus-6 wrote: done. https://github.com/llvm/llvm-project/pull/74008 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[compiler-rt] [llvm] [clang] [clang-tools-extra] [PGO][GlobalValue][LTO]In GlobalValues::getGlobalIdentifier, use semicolon as delimiter for local-linkage varibles. (PR #74008)
@@ -147,6 +147,9 @@ def exclude_unsupported_files_for_aix(dirname): config.substitutions.append( ("%clang_pgouse=", build_invocation(clang_cflags) + " -fprofile-use=") ) +config.substitutions.append( +("%clangxx_pgouse=", build_invocation(clang_cxxflags) + " -fprofile-use=") minglotus-6 wrote: yes, this is used in the new compiler-rt test. Previously there is `clang_pgouse` but no `clangxx_pgouse`. https://github.com/llvm/llvm-project/pull/74008 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[llvm] [clang-tools-extra] [clang] [compiler-rt] [PGO][GlobalValue][LTO]In GlobalValues::getGlobalIdentifier, use semicolon as delimiter for local-linkage varibles. (PR #74008)
@@ -0,0 +1,97 @@ +// This is a regression test for ThinLTO indirect-call-promotion when candidate +// callees need to be imported from another IR module. In the C++ test case, +// `main` calls `global_func` which is defined in another module. `global_func` +// has two indirect callees, one has external linkage and one has local linkage. +// All three functions should be imported into the IR module of main. + +// What the test does: +// - Generate raw profiles from executables and convert it to indexed profiles. +// During the conversion, a profiled callee address in raw profiles will be +// converted to function hash in indexed profiles. +// - Run IRPGO profile use and ThinTLO prelink pipeline and get LLVM bitcodes +// for both cpp files in the C++ test case. +// - Generate ThinLTO summary file with LLVM bitcodes, and run `function-import` pass. +// - Run `pgo-icall-prom` pass for the IR module which needs to import callees. + +// RUN: rm -rf %t && split-file %s %t && cd %t + +// Use clang*_{pgogen,pgouse} for IR level instrumentation, and use clangxx* for +// C++. +// +// Do setup work for all below tests. +// Generate raw profiles from real programs and convert it into indexed profiles. +// RUN: %clangxx_pgogen -fuse-ld=lld -O2 lib.cpp main.cpp -o main minglotus-6 wrote: done. https://github.com/llvm/llvm-project/pull/74008 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[compiler-rt] [clang-tools-extra] [llvm] [clang] [PGO][GlobalValue][LTO]In GlobalValues::getGlobalIdentifier, use semicolon as delimiter for local-linkage varibles. (PR #74008)
@@ -0,0 +1,97 @@ +// This is a regression test for ThinLTO indirect-call-promotion when candidate +// callees need to be imported from another IR module. In the C++ test case, +// `main` calls `global_func` which is defined in another module. `global_func` +// has two indirect callees, one has external linkage and one has local linkage. +// All three functions should be imported into the IR module of main. + +// What the test does: +// - Generate raw profiles from executables and convert it to indexed profiles. +// During the conversion, a profiled callee address in raw profiles will be +// converted to function hash in indexed profiles. +// - Run IRPGO profile use and ThinTLO prelink pipeline and get LLVM bitcodes +// for both cpp files in the C++ test case. +// - Generate ThinLTO summary file with LLVM bitcodes, and run `function-import` pass. +// - Run `pgo-icall-prom` pass for the IR module which needs to import callees. + +// RUN: rm -rf %t && split-file %s %t && cd %t + +// Use clang*_{pgogen,pgouse} for IR level instrumentation, and use clangxx* for +// C++. +// +// Do setup work for all below tests. +// Generate raw profiles from real programs and convert it into indexed profiles. +// RUN: %clangxx_pgogen -fuse-ld=lld -O2 lib.cpp main.cpp -o main +// RUN: env LLVM_PROFILE_FILE=main.profraw %run ./main +// RUN: llvm-profdata merge main.profraw -o main.profdata + +// Use profile on lib and get bitcode, test that local function callee0 has +// expected !PGOFuncName metadata and external function callee1 doesn't have +// !PGOFuncName metadata. Explicitly skip ICP pass to test ICP happens as +// expected in the IR module that imports functions from lib. +// RUN: %clang -target x86_64-unknown-linux-gnu -mllvm -disable-icp -fprofile-use=main.profdata -flto=thin -O2 -c lib.cpp -o lib.bc +// RUN: llvm-dis lib.bc -o - | FileCheck %s --check-prefix=PGOName + +// Use profile on main and get bitcode. +// RUN: %clang -target x86_64-unknown-linux-gnu -fprofile-use=main.profdata -flto=thin -O2 -c main.cpp -o main.bc + +// Run llvm-lto to get summary file. +// RUN: llvm-lto -thinlto -o summary main.bc lib.bc + +// Test the imports of functions. Default import thresholds would work but do +// explicit override to be more futureproof. Note all functions have one basic +// block with a function-entry-count of one, so they are actually hot functions +// per default profile summary hotness cutoff. +// RUN: opt -passes=function-import -import-instr-limit=100 -import-cold-multiplier=1 -summary-file summary.thinlto.bc main.bc -o main.import.bc -print-imports 2>&1 | FileCheck %s --check-prefix=IMPORTS +// Test that '_Z11global_funcv' has indirect calls annotated with value profiles. +// RUN: llvm-dis main.import.bc -o - | FileCheck %s --check-prefix=IR + +// Test that both candidates are ICP'ed and there is no `!VP` in the IR. +// RUN: opt main.import.bc -icp-lto -passes=pgo-icall-prom -S -pass-remarks=pgo-icall-prom 2>&1 | FileCheck %s --check-prefixes=ICP-IR,ICP-REMARK --implicit-check-not="!VP" + +// IMPORTS: main.cpp: Import _Z7callee1v +// IMPORTS: main.cpp: Import _ZL7callee0v.llvm.{{[0-9]+}} +// IMPORTS: main.cpp: Import _Z11global_funcv + +// PGOName: define dso_local void @_Z7callee1v() {{.*}} !prof !{{[0-9]+}} { +// PGOName: define internal void @_ZL7callee0v() {{.*}} !prof !{{[0-9]+}} !PGOFuncName ![[MD:[0-9]+]] { +// PGOName: ![[MD]] = !{!"{{.*}}lib.cpp;_ZL7callee0v"} + +// IR-LABEL: define available_externally {{.*}} void @_Z11global_funcv() {{.*}} !prof !35 { minglotus-6 wrote: thanks for the catch! I updated it to use numerical matcher `[[#]]` https://github.com/llvm/llvm-project/pull/74008 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [compiler-rt] [clang-tools-extra] [llvm] [PGO][GlobalValue][LTO]In GlobalValues::getGlobalIdentifier, use semicolon as delimiter for local-linkage varibles. (PR #74008)
@@ -1,39 +0,0 @@ -; Do setup work for all below tests: generate bitcode and combined index minglotus-6 wrote: this is a valid point. Add the test back (in the new commit), use `rm -rf %t && split-file %s %t && cd %t` on IR and apply the `trap-on-exit` clean-up on the shell script. https://github.com/llvm/llvm-project/pull/74008 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[compiler-rt] [clang-tools-extra] [llvm] [clang] [PGO][GlobalValue][LTO]In GlobalValues::getGlobalIdentifier, use semicolon as delimiter for local-linkage varibles. (PR #74008)
@@ -0,0 +1,97 @@ +// This is a regression test for ThinLTO indirect-call-promotion when candidate +// callees need to be imported from another IR module. In the C++ test case, +// `main` calls `global_func` which is defined in another module. `global_func` +// has two indirect callees, one has external linkage and one has local linkage. +// All three functions should be imported into the IR module of main. + +// What the test does: +// - Generate raw profiles from executables and convert it to indexed profiles. +// During the conversion, a profiled callee address in raw profiles will be +// converted to function hash in indexed profiles. +// - Run IRPGO profile use and ThinTLO prelink pipeline and get LLVM bitcodes +// for both cpp files in the C++ test case. +// - Generate ThinLTO summary file with LLVM bitcodes, and run `function-import` pass. +// - Run `pgo-icall-prom` pass for the IR module which needs to import callees. + +// RUN: rm -rf %t && split-file %s %t && cd %t + +// Use clang*_{pgogen,pgouse} for IR level instrumentation, and use clangxx* for +// C++. +// +// Do setup work for all below tests. +// Generate raw profiles from real programs and convert it into indexed profiles. +// RUN: %clangxx_pgogen -fuse-ld=lld -O2 lib.cpp main.cpp -o main minglotus-6 wrote: I ended up removing `-fuse-ld=lld` in this test. https://github.com/llvm/llvm-project/pull/74008 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[compiler-rt] [llvm] [clang] [clang-tools-extra] [PGO][GlobalValue][LTO]In GlobalValues::getGlobalIdentifier, use semicolon as delimiter for local-linkage varibles. (PR #74008)
@@ -147,6 +147,9 @@ def exclude_unsupported_files_for_aix(dirname): config.substitutions.append( ("%clang_pgouse=", build_invocation(clang_cflags) + " -fprofile-use=") ) +config.substitutions.append( +("%clangxx_pgouse=", build_invocation(clang_cxxflags) + " -fprofile-use=") minglotus-6 wrote: oh you are correct. I once used `clangxx_pgouse` in the new test for profile use, but later thought `clang -c` (run up to assembler phase to generate bitcode) together with `opt` and `llvm-lto` is better. https://github.com/llvm/llvm-project/pull/74008 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[llvm] [clang] [clang-tools-extra] [compiler-rt] [PGO][GlobalValue][LTO]In GlobalValues::getGlobalIdentifier, use semicolon as delimiter for local-linkage varibles. (PR #74008)
@@ -0,0 +1,97 @@ +// This is a regression test for ThinLTO indirect-call-promotion when candidate +// callees need to be imported from another IR module. In the C++ test case, +// `main` calls `global_func` which is defined in another module. `global_func` +// has two indirect callees, one has external linkage and one has local linkage. +// All three functions should be imported into the IR module of main. + +// What the test does: +// - Generate raw profiles from executables and convert it to indexed profiles. +// During the conversion, a profiled callee address in raw profiles will be +// converted to function hash in indexed profiles. +// - Run IRPGO profile use and ThinTLO prelink pipeline and get LLVM bitcodes +// for both cpp files in the C++ test case. +// - Generate ThinLTO summary file with LLVM bitcodes, and run `function-import` pass. +// - Run `pgo-icall-prom` pass for the IR module which needs to import callees. + +// RUN: rm -rf %t && split-file %s %t && cd %t + +// Use clang*_{pgogen,pgouse} for IR level instrumentation, and use clangxx* for +// C++. +// +// Do setup work for all below tests. +// Generate raw profiles from real programs and convert it into indexed profiles. +// RUN: %clangxx_pgogen -fuse-ld=lld -O2 lib.cpp main.cpp -o main minglotus-6 wrote: I see. This makes sense to me. Done. https://github.com/llvm/llvm-project/pull/74008 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [compiler-rt] [llvm] [clang-tools-extra] [PGO][GlobalValue][LTO]In GlobalValues::getGlobalIdentifier, use semicolon as delimiter for local-linkage varibles. (PR #74008)
@@ -0,0 +1,115 @@ +// This is a regression test for ThinLTO indirect-call-promotion when candidate +// callees need to be imported from another IR module. In the C++ test case, +// `main` calls `global_func` which is defined in another module. `global_func` +// has two indirect callees, one has external linkage and one has local linkage. +// All three functions should be imported into the IR module of main. + +// What the test does: +// - Generate raw profiles from executables and convert it to indexed profiles. +// During the conversion, a profiled callee address in raw profiles will be +// converted to function hash in indexed profiles. +// - Run IRPGO profile use and ThinTLO prelink pipeline and get LLVM bitcodes +// for both cpp files in the C++ test case. +// - Generate ThinLTO summary file with LLVM bitcodes, and run `function-import` pass. +// - Run `pgo-icall-prom` pass for the IR module which needs to import callees. + +// Use lld as linker for more robust test. We need to REQUIRE LLVMgold.so for +// LTO if default linker is GNU ld or gold anyway. +// REQUIRES: lld-available + +// Test should fail where linkage-name and mangled-name diverges, see issue https://github.com/llvm/llvm-project/issues/74565). +// Currently, this name divergence happens on Mach-O object file format, or on +// many (but not all) 32-bit Windows systems. minglotus-6 wrote: Some more details regarding this statement 1. The global prefix is `_` for {`MachO`, `WinCOFFX86`} and `\0` (i.e., no prefix) for the rest of mangling modes ([source code](https://github.com/llvm/llvm-project/blob/185302530847a28f44e48a67a79fd4eba048a1c7/llvm/include/llvm/IR/DataLayout.h#L316-L328)). * The global prefix is [used by IR Mangler](https://github.com/llvm/llvm-project/blob/62b21c6ced918c7fec97b557e3087e3ffdf71494/llvm/lib/IR/Mangler.cpp#L144) in global symbols (e.g., prefix [emitted](https://github.com/llvm/llvm-project/blob/62b21c6ced918c7fec97b557e3087e3ffdf71494/llvm/lib/IR/Mangler.cpp#L55-L56) if it's not `\0`) 2. In the data-layout string,`m:o` specifies `MachO`, `m:x` specifies `WinCOFFX86`, and `m:e` specifies `ELF` ([parser source code](https://github.com/llvm/llvm-project/blob/185302530847a28f44e48a67a79fd4eba048a1c7/llvm/lib/IR/DataLayout.cpp#L510-L541)) With 1 and 2, note not all 32-bit windows uses `WinCOFFX86` mangling-mode. For instance, one windows-32 target [specifies](https://github.com/llvm/llvm-project/blob/a8ef9c0969ab0807672bcad60970bf22395ffaf1/clang/lib/Basic/Targets/X86.h#L672) `m:e`, and some [1] windows-32 target chooses between `m:e` and one of {`m:x`, `m:o`} [1] [example 1](https://github.com/llvm/llvm-project/blob/a8ef9c0969ab0807672bcad60970bf22395ffaf1/clang/lib/Basic/Targets/X86.h#L581) chooses between `m:x` and `m:e`,and [example 2](https://github.com/llvm/llvm-project/blob/a8ef9c0969ab0807672bcad60970bf22395ffaf1/clang/lib/Basic/Targets/X86.h#L444-L447) chooses between `m:o` and `m:e` https://github.com/llvm/llvm-project/pull/74008 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[llvm] [clang-tools-extra] [clang] [nfc][llvm-profdata] Use cl::Subcommand to organize subcommand and options in llvm-profdata (PR #71328)
https://github.com/minglotus-6 closed https://github.com/llvm/llvm-project/pull/71328 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang] [llvm] [compiler-rt] [IRPGO][ValueProfile] Instrument virtual table address that could be used to do virtual table address comparision for indirect-call-promotion. (PR #668
@@ -12,27 +12,78 @@ #ifndef LLVM_ANALYSIS_INDIRECTCALLVISITOR_H #define LLVM_ANALYSIS_INDIRECTCALLVISITOR_H +#include "llvm/ADT/SetVector.h" #include "llvm/IR/InstVisitor.h" #include namespace llvm { -// Visitor class that finds all indirect call. +// Visitor class that finds indirect calls or instructions that gives vtable +// value, depending on Type. struct PGOIndirectCallVisitor : public InstVisitor { + enum class InstructionType { +kIndirectCall = 0, +kVTableVal = 1, + }; std::vector IndirectCalls; - PGOIndirectCallVisitor() = default; + std::vector ProfiledAddresses; + PGOIndirectCallVisitor(InstructionType Type) : Type(Type) {} void visitCallBase(CallBase &Call) { -if (Call.isIndirectCall()) +if (Call.isIndirectCall()) { IndirectCalls.push_back(&Call); + + if (Type != InstructionType::kVTableVal) +return; + + LoadInst *LI = dyn_cast(Call.getCalledOperand()); + // The code pattern to look for + // + // %vtable = load ptr, ptr %b + // %vfn = getelementptr inbounds ptr, ptr %vtable, i64 1 + // %2 = load ptr, ptr %vfn + // %call = tail call i32 %2(ptr %b) + // + // %vtable is the vtable address value to profile, and + // %2 is the indirect call target address to profile. + if (LI != nullptr) { +Value *Ptr = LI->getPointerOperand(); +Value *VTablePtr = Ptr->stripInBoundsConstantOffsets(); +// This is a heuristic to find address feeding instructions. +// FIXME: Add support in the frontend so LLVM type intrinsics are +// emitted without LTO. This way, added intrinsics could filter +// non-vtable instructions and reduce instrumentation overhead. +// Since a non-vtable profiled address is not within the address +// range of vtable objects, it's stored as zero in indexed profiles. minglotus-6 wrote: The stored zero values would be dropped by optimizers currently. Dropping values at profile post-processing time might be possible; yet it requires some implementation changes. An alternative is to improve the accuracy of vtable instrumentation (e.g., make `clang` insert type intrinsic functions with FDO without requiring `-flto` at instrumentation phase) as follow-up work so we could get fewer false positives and faster instrumented binary. https://github.com/llvm/llvm-project/pull/66825 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clang] [llvm] [compiler-rt] [IRPGO][ValueProfile] Instrument virtual table address that could be used to do virtual table address comparision for indirect-call-promotion. (PR #668
@@ -490,6 +591,23 @@ Error InstrProfSymtab::addFuncWithName(Function &F, StringRef PGOFuncName) { return Error::success(); } +uint64_t InstrProfSymtab::getVTableHashFromAddress(uint64_t Address) { + finalizeSymtab(); + auto It = lower_bound( + VTableAddrRangeToMD5Map, Address, minglotus-6 wrote: New test case added in `value_prof_data_read_write_mapping` (in InstrProfTest.cpp) catches a subtle bug by `VTableRangeAddr.first.second < Addr`. Already fixed by using `VTableRangeAddr.first.second <= Addr`. Basically, the profiled address from a vtable should be in the range [VTableStartAddr, VTableEndAddr). In the unit test, each vtable has one function (no offset-to-top, no RTTI). Using `<` would map profiled address to the wrong vtable. https://github.com/llvm/llvm-project/pull/66825 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[llvm] [clang-tools-extra] [clang] [compiler-rt] [IRPGO][ValueProfile] Instrument virtual table address that could be used to do virtual table address comparision for indirect-call-promotion. (PR #668
@@ -1441,6 +1531,9 @@ void OverlapStats::dump(raw_fd_ostream &OS) const { case IPVK_MemOPSize: strncpy(ProfileKindName, "MemOP", 19); break; +case IPVK_VTableTarget: + strncpy(ProfileKindName, "VTable", 6); minglotus-6 wrote: thanks for the catch! I updated this to `strncpy(ProfileKindName, "VTable", 19);`. Updated the initializer to `ProfileKindName[20] = {0}` to make it clearer. - C string literals are [null terminated](https://softwareengineering.stackexchange.com/questions/344603/are-c-strings-always-null-terminated-or-does-it-depend-on-the-platform), and `strncpy` will [pad destination will zeros if the end of source is seen](https://cplusplus.com/reference/cstring/strncpy/) before 19 chars are copied here. I just realized '19' is chosen as 'sizeof(ProfileKindName) - 1' with this comment. https://github.com/llvm/llvm-project/pull/66825 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[compiler-rt] [llvm] [clang] [clang-tools-extra] [IRPGO][ValueProfile] Instrument virtual table address that could be used to do virtual table address comparision for indirect-call-promotion. (PR #668
https://github.com/minglotus-6 edited https://github.com/llvm/llvm-project/pull/66825 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[llvm] [clang-tools-extra] [compiler-rt] [clang] [IRPGO][ValueProfile] Instrument virtual table address that could be used to do virtual table address comparision for indirect-call-promotion. (PR #668
https://github.com/minglotus-6 commented: thanks for the reviews and discussions! This PR currently merged upstream main at a commit on Nov 9th. I started to have local changes after that, so refrained from another merge. Will do that separately. https://github.com/llvm/llvm-project/pull/66825 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[compiler-rt] [clang-tools-extra] [clang] [llvm] [IRPGO][ValueProfile] Instrument virtual table address that could be used to do virtual table address comparision for indirect-call-promotion. (PR #668
@@ -123,19 +137,29 @@ static int needsCounterPadding(void) { COMPILER_RT_VISIBILITY void __llvm_profile_get_padding_sizes_for_counters( uint64_t DataSize, uint64_t CountersSize, uint64_t NumBitmapBytes, -uint64_t NamesSize, uint64_t *PaddingBytesBeforeCounters, -uint64_t *PaddingBytesAfterCounters, uint64_t *PaddingBytesAfterBitmapBytes, -uint64_t *PaddingBytesAfterNames) { +uint64_t NamesSize, uint64_t VTableSize, uint64_t VNameSize, +uint64_t *PaddingBytesBeforeCounters, uint64_t *PaddingBytesAfterCounters, +uint64_t *PaddingBytesAfterBitmapBytes, uint64_t *PaddingBytesAfterNames, +uint64_t *PaddingBytesAfterVTable, uint64_t *PaddingBytesAfterVName) { + // Counter padding is needed only if continuous mode is enabled. if (!needsCounterPadding()) { *PaddingBytesBeforeCounters = 0; *PaddingBytesAfterCounters = __llvm_profile_get_num_padding_bytes(CountersSize); *PaddingBytesAfterBitmapBytes = __llvm_profile_get_num_padding_bytes(NumBitmapBytes); *PaddingBytesAfterNames = __llvm_profile_get_num_padding_bytes(NamesSize); +if (PaddingBytesAfterVTable != NULL) + *PaddingBytesAfterVTable = + __llvm_profile_get_num_padding_bytes(VTableSize); +if (PaddingBytesAfterVName != NULL) + *PaddingBytesAfterVName = __llvm_profile_get_num_padding_bytes(VNameSize); return; } + // Value profiling not supported in continuous mode at profile-write time. + assert(VTableSize == 0 && VNameSize == 0 && minglotus-6 wrote: As discussed offline, changed this function to return -1 upon errors and return 0 upon success, and updated the caller `lprofWriteDataImpl` to propagated the error (if any) up along the call chain -> currently, the error is handled [at this place](https://github.com/llvm/llvm-project/blob/250d9c86c201799755611c425ce6e02fb5867716/compiler-rt/lib/profile/InstrProfilingFile.c#L1116) to emit " LLVM Profile Error: Failed to write file ..." https://github.com/llvm/llvm-project/pull/66825 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[compiler-rt] [llvm] [clang-tools-extra] [clang] [IRPGO][ValueProfile] Instrument virtual table address that could be used to do virtual table address comparision for indirect-call-promotion. (PR #668
@@ -275,18 +282,28 @@ lprofWriteDataImpl(ProfDataWriter *Writer, const __llvm_profile_data *DataBegin, const uint64_t NumBitmapBytes = __llvm_profile_get_num_bitmap_bytes(BitmapBegin, BitmapEnd); const uint64_t NamesSize = __llvm_profile_get_name_size(NamesBegin, NamesEnd); + const uint64_t NumVTables = + __llvm_profile_get_num_vtable(VTableBegin, VTableEnd); + const uint64_t VTableSectionSize = + __llvm_profile_get_vtable_section_size(VTableBegin, VTableEnd); + // Value profiling is not supported when DebugInfoCorrelate is true. minglotus-6 wrote: This comment is likely cause more confusion than clearing them. I deleted it. In the initial commit, the code and comment are like this ``` // Note, in reality, vtable profiling is not supported when DebugInfoCorrelate // is true. const uint64_t VNamesSize = DebugInfoCorrelate ? 0 : VNamesEnd - VNamesBegin; ``` By then, the comment explains the reason (i.e., value profiling won't happen in the first place when `DebugInfoCorrelate` is true according to [this comment](https://github.com/llvm/llvm-project/blob/72552fc5cbc0cf2f44508948a075d14f0d5aa2b3/compiler-rt/lib/profile/InstrProfilingWriter.c#L334-L335)) to set `VNamesSize` to `0` when `DebugInfoCorrelate` is true. Now [__llvm_profile_get_name_size](https://github.com/llvm/llvm-project/blob/72552fc5cbc0cf2f44508948a075d14f0d5aa2b3/compiler-rt/lib/profile/InstrProfilingBuffer.c#L100-L104) handles the conditional zero so deleted the comment. https://github.com/llvm/llvm-project/pull/66825 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[compiler-rt] [clang] [clang-tools-extra] [llvm] [IRPGO][ValueProfile] Instrument virtual table address that could be used to do virtual table address comparision for indirect-call-promotion. (PR #668
@@ -489,30 +520,66 @@ class InstrProfSymtab { /// \p IterRange. This interface is used by IndexedProfReader. template Error create(const NameIterRange &IterRange); - /// Update the symtab by adding \p FuncName to the table. This interface - /// is used by the raw and text profile readers. - Error addFuncName(StringRef FuncName) { -if (FuncName.empty()) + /// Create InstrProfSymtab from a set of function names and vtable + /// names iteratable from \p IterRange. This interface is used by + /// IndexedProfReader. + template + Error create(const FuncNameIterRange &FuncIterRange, + const VTableNameIterRange &VTableIterRange); + + Error addSymbolName(StringRef SymbolName) { +if (SymbolName.empty()) return make_error(instrprof_error::malformed, -"function name is empty"); -auto Ins = NameTab.insert(FuncName); +"symbol name is empty"); + +// Insert into NameTab so that MD5NameMap (a vector that will be sorted) +// won't have duplicated entries in the first place. +auto Ins = NameTab.insert(SymbolName); if (Ins.second) { MD5NameMap.push_back(std::make_pair( - IndexedInstrProf::ComputeHash(FuncName), Ins.first->getKey())); + IndexedInstrProf::ComputeHash(SymbolName), Ins.first->getKey())); Sorted = false; } return Error::success(); } + /// The method name is kept since there are many callers. + /// It just forwards to 'addSymbolName'. + Error addFuncName(StringRef FuncName) { return addSymbolName(FuncName); } + + /// Adds VTableName as a known symbol, and inserts it to a map that + /// tracks all vtable names. + Error addVTableName(StringRef VTableName) { +if (Error E = addSymbolName(VTableName)) + return E; + +// Record VTableName. InstrProfWriter uses this map. The comment around +// class member explains why. +VTableNames.insert(VTableName); +return Error::success(); + } + + const StringSet<> &getVTableNames() const { return VTableNames; } + /// Map a function address to its name's MD5 hash. This interface /// is only used by the raw profiler reader. void mapAddress(uint64_t Addr, uint64_t MD5Val) { AddrToMD5Map.push_back(std::make_pair(Addr, MD5Val)); } + /// Map the address range (i.e., [start_address, end_address]) of a variable to + /// its names' MD5 hash. This interface is only used by the raw profile header. minglotus-6 wrote: yup it should be header. I fixed this. https://github.com/llvm/llvm-project/pull/66825 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [llvm] [compiler-rt] [clang] [IRPGO][ValueProfile] Instrument virtual table address that could be used to do virtual table address comparision for indirect-call-promotion. (PR #668
@@ -453,11 +471,94 @@ Error InstrProfSymtab::create(Module &M, bool InLTO) { if (Error E = addFuncWithName(F, getPGOFuncName(F, InLTO))) return E; } + + SmallVector Types; + for (GlobalVariable &G : M.globals()) { +if (!G.hasName()) + continue; +Types.clear(); +G.getMetadata(LLVMContext::MD_type, Types); +if (!Types.empty()) { + MD5VTableMap.emplace_back(G.getGUID(), &G); +} + } Sorted = false; finalizeSymtab(); return Error::success(); } +/// \c NameStrings is a string composed of one of more possibly encoded +/// sub-strings. The substrings are separated by 0 or more zero bytes. This +/// method decodes the string and calls `NameCallback` for each substring. +static Error +readAndDecodeStrings(StringRef NameStrings, minglotus-6 wrote: Good point. Updated InstrProfTest.cpp to have test coverage for the added value type. There are quite some common code for indirect_call and vtable test coverage, so factor them out. I could split the unit-test refactor as a pre-commit of this PR if that's better, but thought to put them here first for the whole picture how they are used. Test case construction catches an edge case (see the comment in `InstrProfSymtab::getVTableHashFromAddress`), which should be rare at least under Itanium ABI. https://github.com/llvm/llvm-project/pull/66825 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[llvm] [clang] [clang-tools-extra] [compiler-rt] [IRPGO][ValueProfile] Instrument virtual table address that could be used to do virtual table address comparision for indirect-call-promotion. (PR #668
@@ -1070,8 +1084,138 @@ void InstrProfiling::maybeSetComdat(GlobalVariable *GV, Function *Fn, GV->setLinkage(GlobalValue::InternalLinkage); } -GlobalVariable *InstrProfiling::setupProfileSection(InstrProfInstBase *Inc, -InstrProfSectKind IPSK) { +static inline bool shouldRecordVTableAddr(GlobalVariable *GV) { + if (!profDataReferencedByCode(*GV->getParent())) +return false; + + if (!GV->hasLinkOnceLinkage() && !GV->hasLocalLinkage() && + !GV->hasAvailableExternallyLinkage()) +return true; + + // This avoids the profile data from referencing internal symbols in + // COMDAT. + if (GV->hasLocalLinkage() && GV->hasComdat()) +return false; + + return true; +} + +// FIXME: Does symbollic relocation from 'getFuncAddrForProfData' matter here? minglotus-6 wrote: fixed this! https://github.com/llvm/llvm-project/pull/66825 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[llvm] [compiler-rt] [clang] [clang-tools-extra] [IRPGO][ValueProfile] Instrument virtual table address that could be used to do virtual table address comparision for indirect-call-promotion. (PR #668
@@ -1070,8 +1084,138 @@ void InstrProfiling::maybeSetComdat(GlobalVariable *GV, Function *Fn, GV->setLinkage(GlobalValue::InternalLinkage); } -GlobalVariable *InstrProfiling::setupProfileSection(InstrProfInstBase *Inc, -InstrProfSectKind IPSK) { +static inline bool shouldRecordVTableAddr(GlobalVariable *GV) { + if (!profDataReferencedByCode(*GV->getParent())) +return false; + + if (!GV->hasLinkOnceLinkage() && !GV->hasLocalLinkage() && + !GV->hasAvailableExternallyLinkage()) +return true; + + // This avoids the profile data from referencing internal symbols in + // COMDAT. + if (GV->hasLocalLinkage() && GV->hasComdat()) +return false; + + return true; +} + +// FIXME: Does symbollic relocation from 'getFuncAddrForProfData' matter here? +static inline Constant *getVTableAddrForProfData(GlobalVariable *GV) { + auto *Int8PtrTy = Type::getInt8PtrTy(GV->getContext()); + + // Store a nullptr in __profvt_ if a real address shouldn't be used. + if (!shouldRecordVTableAddr(GV)) +return ConstantPointerNull::get(Int8PtrTy); + + return ConstantExpr::getBitCast(GV, Int8PtrTy); +} + +/// Get the name of a profiling variable for a particular variable. +static std::string getVarName(GlobalVariable *GV, StringRef Prefix) { + StringRef Name = getPGOName(*GV); + return (Prefix + Name).str(); +} + +void InstrProfiling::getOrCreateVTableProfData(GlobalVariable *GV) { + assert(!DebugInfoCorrelate && + "Value profiling is not supported with lightweight instrumentation"); + if (GV->isDeclaration() || GV->hasAvailableExternallyLinkage()) +return; + + if (GV->getName().starts_with("llvm.") || + GV->getName().starts_with("__llvm") || + GV->getName().starts_with("__prof")) +return; + + // VTableProfData already created + auto It = VTableDataMap.find(GV); + if (It != VTableDataMap.end() && It->second) +return; + + GlobalValue::LinkageTypes Linkage = GV->getLinkage(); + GlobalValue::VisibilityTypes Visibility = GV->getVisibility(); + + // This is to keep consistent with per-function profile data + // for correctness. + if (TT.isOSBinFormatXCOFF()) { +Linkage = GlobalValue::InternalLinkage; +Visibility = GlobalValue::DefaultVisibility; + } + + LLVMContext &Ctx = M->getContext(); + Type *DataTypes[] = { +#define INSTR_PROF_VTABLE_DATA(Type, LLVMType, Name, Init) LLVMType, +#include "llvm/ProfileData/InstrProfData.inc" + }; + + auto *DataTy = StructType::get(Ctx, ArrayRef(DataTypes)); + + // Used by INSTR_PROF_VTABLE_DATA MACRO + Constant *VTableAddr = getVTableAddrForProfData(GV); + StringRef VTableName = GV->getName(); + StringRef PGOVTableName = getPGOName(*GV); + // Record the length of the vtable. This is needed since vtable pointers + // loaded from C++ objects might be from the middle of a vtable definition. + uint32_t VTableSizeVal = + M->getDataLayout().getTypeAllocSize(GV->getValueType()); + + Constant *DataVals[] = { +#define INSTR_PROF_VTABLE_DATA(Type, LLVMType, Name, Init) Init, +#include "llvm/ProfileData/InstrProfData.inc" + }; + + auto *Data = + new GlobalVariable(*M, DataTy, false /* constant */, Linkage, + ConstantStruct::get(DataTy, DataVals), + getVarName(GV, getInstrProfVTableVarPrefix())); + + Data->setVisibility(Visibility); + Data->setSection(getInstrProfSectionName(IPSK_vtab, TT.getObjectFormat())); + Data->setAlignment(Align(8)); + + const bool NeedComdat = needsComdatForCounter(*GV, *M); + + // GV is the data structure to record vtable information. + // Place the global variable for per-vtable profile data in a comdat group + // if the associated vtable definition is a COMDAT. This makes sure only one + // copy of the variable for the vtable will be emitted after linking. + auto MaybeSetComdat = [&](GlobalVariable *GV, StringRef GroupName) { +bool UseComdat = (NeedComdat || TT.isOSBinFormatELF()); +if (UseComdat) { + // Create a new comdat group using the name of the global variable as + // opposed to using the comdat group of the vtable. + Comdat *C = M->getOrInsertComdat(GroupName); + // For ELF, when not using COMDAT, put the vtable profile data into a + // nodeduplicate COMDAT which is lowered to a zero-flag zero group. + // This allows -z -start-top-gc to discard the entire group when the minglotus-6 wrote: done. https://github.com/llvm/llvm-project/pull/66825 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[compiler-rt] [llvm] [clang-tools-extra] [clang] [IRPGO][ValueProfile] Instrument virtual table address that could be used to do virtual table address comparision for indirect-call-promotion. (PR #668
minglotus-6 wrote: One heads up, the latest 'git merge main' brings a failure in test https://github.com/llvm/llvm-project/blob/main/compiler-rt/test/fuzzer/gc-sections.test :( . `ninja check-all` on a clean main gives the same failure. Hopefully it would be fixed soon. https://github.com/llvm/llvm-project/pull/66825 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[lldb] [clang] [mlir] [compiler-rt] [libc] [flang] [libunwind] [llvm] [lld] [libcxx] [clang-tools-extra] [IRPGO][ValueProfile] Instrument virtual table address that could be used to do virtual table a
minglotus-6 wrote: > One heads up, the latest 'git merge main' brings a failure in test > https://github.com/llvm/llvm-project/blob/main/compiler-rt/test/fuzzer/gc-sections.test > :( . `ninja check-all` on a clean main gives the same failure. Hopefully it > would be fixed soon. This is fixed at upstream main so merged upstream main. https://github.com/llvm/llvm-project/pull/66825 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[lld] [llvm] [lldb] [clang-tools-extra] [libcxx] [libunwind] [mlir] [libc] [clang] [compiler-rt] [flang] [IRPGO][ValueProfile] Instrument virtual table address that could be used to do virtual table a
@@ -453,11 +471,94 @@ Error InstrProfSymtab::create(Module &M, bool InLTO) { if (Error E = addFuncWithName(F, getPGOFuncName(F, InLTO))) return E; } + + SmallVector Types; + for (GlobalVariable &G : M.globals()) { +if (!G.hasName()) + continue; +Types.clear(); +G.getMetadata(LLVMContext::MD_type, Types); +if (!Types.empty()) { + MD5VTableMap.emplace_back(G.getGUID(), &G); +} + } Sorted = false; finalizeSymtab(); return Error::success(); } +/// \c NameStrings is a string composed of one of more possibly encoded +/// sub-strings. The substrings are separated by 0 or more zero bytes. This +/// method decodes the string and calls `NameCallback` for each substring. +static Error +readAndDecodeStrings(StringRef NameStrings, minglotus-6 wrote: Since InstrProfTest.cpp changes a lot, created a [nfc patch](https://github.com/llvm/llvm-project/pull/72611/files) to prepare for test coverage of a new type of value profiles. https://github.com/llvm/llvm-project/pull/66825 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [IRPGO][ValueProfile] Instrument virtual table address that could be used to do virtual table address comparision for indirect-call-promotion. (PR #66825)
@@ -18,11 +18,16 @@ * pointers to the live data in memory. This function is probably not what you * want. Use __llvm_profile_get_size_for_buffer instead. Use this function if * your program has a custom memory layout. + * NOTE: The change of function signature requires modifying c source code + * as demonstrated by the existing tests. If this is causing backward + * compatible issues, considering adding another function for new use cases. */ uint64_t __llvm_profile_get_size_for_buffer_internal( const __llvm_profile_data *DataBegin, const __llvm_profile_data *DataEnd, const char *CountersBegin, const char *CountersEnd, const char *NamesBegin, -const char *NamesEnd); +const char *NamesEnd, const VTableProfData *VTableBegin, minglotus-6 wrote: This shouldn't not be used except for online profile merge (where value profile is not supported), so reverted the interface change (and the change in `compiler-rt/test/profile/instrprof-write-buffer-internal.c`). https://github.com/llvm/llvm-project/pull/66825 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [IRPGO][ValueProfile] Instrument virtual table address that could be used to do virtual table address comparision for indirect-call-promotion. (PR #66825)
https://github.com/minglotus-6 edited https://github.com/llvm/llvm-project/pull/66825 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang]Emit type metadata when -fprofile-generate is on (PR #70841)
https://github.com/minglotus-6 created https://github.com/llvm/llvm-project/pull/70841 None >From 99abebc88c09346bd4a70bbf39df13b249116c09 Mon Sep 17 00:00:00 2001 From: Mingming Liu Date: Tue, 31 Oct 2023 01:17:03 -0700 Subject: [PATCH] [Clang]Emit type metadata when -fprofile-generate is on --- clang/lib/CodeGen/CGVTables.cpp | 2 +- clang/lib/CodeGen/MicrosoftCXXABI.cpp | 3 +- clang/test/CodeGenCXX/type-metadata.cpp | 150 3 files changed, 80 insertions(+), 75 deletions(-) diff --git a/clang/lib/CodeGen/CGVTables.cpp b/clang/lib/CodeGen/CGVTables.cpp index 25e4b1c27932026..895e160dfa8a536 100644 --- a/clang/lib/CodeGen/CGVTables.cpp +++ b/clang/lib/CodeGen/CGVTables.cpp @@ -1312,7 +1312,7 @@ llvm::GlobalObject::VCallVisibility CodeGenModule::GetVCallVisibilityLevel( void CodeGenModule::EmitVTableTypeMetadata(const CXXRecordDecl *RD, llvm::GlobalVariable *VTable, const VTableLayout &VTLayout) { - if (!getCodeGenOpts().LTOUnit) + if (!getCodeGenOpts().LTOUnit && !getCodeGenOpts().hasProfileIRInstr()) return; CharUnits ComponentWidth = GetTargetTypeStoreSize(getVTableComponentType()); diff --git a/clang/lib/CodeGen/MicrosoftCXXABI.cpp b/clang/lib/CodeGen/MicrosoftCXXABI.cpp index d623f8f63ae56c4..740c0b6cb1aac04 100644 --- a/clang/lib/CodeGen/MicrosoftCXXABI.cpp +++ b/clang/lib/CodeGen/MicrosoftCXXABI.cpp @@ -1673,7 +1673,8 @@ void MicrosoftCXXABI::EmitDestructorCall(CodeGenFunction &CGF, void MicrosoftCXXABI::emitVTableTypeMetadata(const VPtrInfo &Info, const CXXRecordDecl *RD, llvm::GlobalVariable *VTable) { - if (!CGM.getCodeGenOpts().LTOUnit) + if (!CGM.getCodeGenOpts().LTOUnit && + !CGM.getCodeGenOpts().hasProfileIRInstr()) return; // TODO: Should VirtualFunctionElimination also be supported here? diff --git a/clang/test/CodeGenCXX/type-metadata.cpp b/clang/test/CodeGenCXX/type-metadata.cpp index 3f8b54af801f8d6..637ec6e6c326ab6 100644 --- a/clang/test/CodeGenCXX/type-metadata.cpp +++ b/clang/test/CodeGenCXX/type-metadata.cpp @@ -1,113 +1,117 @@ // Tests for the cfi-vcall feature: -// RUN: %clang_cc1 -flto -flto-unit -triple x86_64-unknown-linux -fvisibility=hidden -fsanitize=cfi-vcall -fsanitize-trap=cfi-vcall -emit-llvm -o - %s | FileCheck --check-prefix=CFI --check-prefix=CFI-NVT-NO-RV --check-prefix=ITANIUM --check-prefix=ITANIUM-MD --check-prefix=TT-ITANIUM-HIDDEN --check-prefix=NDIAG %s -// RUN: %clang_cc1 -flto -flto-unit -triple x86_64-unknown-linux -fvisibility=hidden -fsanitize=cfi-vcall -emit-llvm -o - %s | FileCheck --check-prefix=CFI --check-prefix=CFI-NVT-NO-RV --check-prefix=ITANIUM --check-prefix=ITANIUM-MD --check-prefix=TT-ITANIUM-HIDDEN --check-prefix=ITANIUM-MD-DIAG --check-prefix=ITANIUM-DIAG --check-prefix=DIAG --check-prefix=DIAG-ABORT %s -// RUN: %clang_cc1 -flto -flto-unit -triple x86_64-unknown-linux -fvisibility=hidden -fsanitize=cfi-vcall -fsanitize-recover=cfi-vcall -emit-llvm -o - %s | FileCheck --check-prefix=CFI --check-prefix=CFI-NVT-NO-RV --check-prefix=ITANIUM --check-prefix=ITANIUM-MD --check-prefix=TT-ITANIUM-HIDDEN --check-prefix=ITANIUM-MD-DIAG --check-prefix=ITANIUM-DIAG --check-prefix=DIAG --check-prefix=DIAG-RECOVER %s -// RUN: %clang_cc1 -flto -flto-unit -triple x86_64-pc-windows-msvc -fsanitize=cfi-vcall -fsanitize-trap=cfi-vcall -emit-llvm -o - %s | FileCheck --check-prefix=CFI --check-prefix=CFI-NVT-NO-RV --check-prefix=MS --check-prefix=TT-MS --check-prefix=NDIAG %s +// RUN: %clang_cc1 -flto -flto-unit -triple x86_64-unknown-linux -fvisibility=hidden -fsanitize=cfi-vcall -fsanitize-trap=cfi-vcall -emit-llvm -o - %s | FileCheck --check-prefix=CFI --check-prefix=CFI-NVT-NO-RV --check-prefix=ITANIUM-TYPEMETADATA --check-prefix=ITANIUM-HIDDEN --check-prefix=ITANIUM-MD --check-prefix=TT-ITANIUM-HIDDEN --check-prefix=NDIAG %s +// RUN: %clang_cc1 -flto -flto-unit -triple x86_64-unknown-linux -fvisibility=hidden -fsanitize=cfi-vcall -emit-llvm -o - %s | FileCheck --check-prefix=CFI --check-prefix=CFI-NVT-NO-RV --check-prefix=ITANIUM-TYPEMETADATA --check-prefix=ITANIUM-HIDDEN --check-prefix=ITANIUM-MD --check-prefix=TT-ITANIUM-HIDDEN --check-prefix=ITANIUM-MD-DIAG --check-prefix=ITANIUM-DIAG --check-prefix=DIAG --check-prefix=DIAG-ABORT %s +// RUN: %clang_cc1 -flto -flto-unit -triple x86_64-unknown-linux -fvisibility=hidden -fsanitize=cfi-vcall -fsanitize-recover=cfi-vcall -emit-llvm -o - %s | FileCheck --check-prefix=CFI --check-prefix=CFI-NVT-NO-RV --check-prefix=ITANIUM-TYPEMETADATA --check-prefix=ITANIUM-HIDDEN --check-prefix=ITANIUM-MD --check-prefix=TT-ITANIUM-HIDDEN --check-prefix=ITANIUM-MD-DIAG --check-prefix=ITANIUM-DIAG --check-prefix=DIAG --check-prefix=DIAG-RECOVER %s +// RUN: %clang_cc1 -flto -flto-unit -triple x86_64-pc-windows-msvc -fsanitize=cfi-vcall -
[clang] [Clang]Emit type metadata when -fprofile-generate is on (PR #70841)
https://github.com/minglotus-6 edited https://github.com/llvm/llvm-project/pull/70841 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang]Emit type metadata when -fprofile-generate is on (PR #70841)
https://github.com/minglotus-6 edited https://github.com/llvm/llvm-project/pull/70841 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang]Emit type metadata when IRPGO is used (PR #70841)
https://github.com/minglotus-6 edited https://github.com/llvm/llvm-project/pull/70841 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang]Emit type metadata when IRPGO is used (PR #70841)
https://github.com/minglotus-6 updated https://github.com/llvm/llvm-project/pull/70841 >From 99abebc88c09346bd4a70bbf39df13b249116c09 Mon Sep 17 00:00:00 2001 From: Mingming Liu Date: Tue, 31 Oct 2023 01:17:03 -0700 Subject: [PATCH 1/2] [Clang]Emit type metadata when -fprofile-generate is on --- clang/lib/CodeGen/CGVTables.cpp | 2 +- clang/lib/CodeGen/MicrosoftCXXABI.cpp | 3 +- clang/test/CodeGenCXX/type-metadata.cpp | 150 3 files changed, 80 insertions(+), 75 deletions(-) diff --git a/clang/lib/CodeGen/CGVTables.cpp b/clang/lib/CodeGen/CGVTables.cpp index 25e4b1c27932026..895e160dfa8a536 100644 --- a/clang/lib/CodeGen/CGVTables.cpp +++ b/clang/lib/CodeGen/CGVTables.cpp @@ -1312,7 +1312,7 @@ llvm::GlobalObject::VCallVisibility CodeGenModule::GetVCallVisibilityLevel( void CodeGenModule::EmitVTableTypeMetadata(const CXXRecordDecl *RD, llvm::GlobalVariable *VTable, const VTableLayout &VTLayout) { - if (!getCodeGenOpts().LTOUnit) + if (!getCodeGenOpts().LTOUnit && !getCodeGenOpts().hasProfileIRInstr()) return; CharUnits ComponentWidth = GetTargetTypeStoreSize(getVTableComponentType()); diff --git a/clang/lib/CodeGen/MicrosoftCXXABI.cpp b/clang/lib/CodeGen/MicrosoftCXXABI.cpp index d623f8f63ae56c4..740c0b6cb1aac04 100644 --- a/clang/lib/CodeGen/MicrosoftCXXABI.cpp +++ b/clang/lib/CodeGen/MicrosoftCXXABI.cpp @@ -1673,7 +1673,8 @@ void MicrosoftCXXABI::EmitDestructorCall(CodeGenFunction &CGF, void MicrosoftCXXABI::emitVTableTypeMetadata(const VPtrInfo &Info, const CXXRecordDecl *RD, llvm::GlobalVariable *VTable) { - if (!CGM.getCodeGenOpts().LTOUnit) + if (!CGM.getCodeGenOpts().LTOUnit && + !CGM.getCodeGenOpts().hasProfileIRInstr()) return; // TODO: Should VirtualFunctionElimination also be supported here? diff --git a/clang/test/CodeGenCXX/type-metadata.cpp b/clang/test/CodeGenCXX/type-metadata.cpp index 3f8b54af801f8d6..637ec6e6c326ab6 100644 --- a/clang/test/CodeGenCXX/type-metadata.cpp +++ b/clang/test/CodeGenCXX/type-metadata.cpp @@ -1,113 +1,117 @@ // Tests for the cfi-vcall feature: -// RUN: %clang_cc1 -flto -flto-unit -triple x86_64-unknown-linux -fvisibility=hidden -fsanitize=cfi-vcall -fsanitize-trap=cfi-vcall -emit-llvm -o - %s | FileCheck --check-prefix=CFI --check-prefix=CFI-NVT-NO-RV --check-prefix=ITANIUM --check-prefix=ITANIUM-MD --check-prefix=TT-ITANIUM-HIDDEN --check-prefix=NDIAG %s -// RUN: %clang_cc1 -flto -flto-unit -triple x86_64-unknown-linux -fvisibility=hidden -fsanitize=cfi-vcall -emit-llvm -o - %s | FileCheck --check-prefix=CFI --check-prefix=CFI-NVT-NO-RV --check-prefix=ITANIUM --check-prefix=ITANIUM-MD --check-prefix=TT-ITANIUM-HIDDEN --check-prefix=ITANIUM-MD-DIAG --check-prefix=ITANIUM-DIAG --check-prefix=DIAG --check-prefix=DIAG-ABORT %s -// RUN: %clang_cc1 -flto -flto-unit -triple x86_64-unknown-linux -fvisibility=hidden -fsanitize=cfi-vcall -fsanitize-recover=cfi-vcall -emit-llvm -o - %s | FileCheck --check-prefix=CFI --check-prefix=CFI-NVT-NO-RV --check-prefix=ITANIUM --check-prefix=ITANIUM-MD --check-prefix=TT-ITANIUM-HIDDEN --check-prefix=ITANIUM-MD-DIAG --check-prefix=ITANIUM-DIAG --check-prefix=DIAG --check-prefix=DIAG-RECOVER %s -// RUN: %clang_cc1 -flto -flto-unit -triple x86_64-pc-windows-msvc -fsanitize=cfi-vcall -fsanitize-trap=cfi-vcall -emit-llvm -o - %s | FileCheck --check-prefix=CFI --check-prefix=CFI-NVT-NO-RV --check-prefix=MS --check-prefix=TT-MS --check-prefix=NDIAG %s +// RUN: %clang_cc1 -flto -flto-unit -triple x86_64-unknown-linux -fvisibility=hidden -fsanitize=cfi-vcall -fsanitize-trap=cfi-vcall -emit-llvm -o - %s | FileCheck --check-prefix=CFI --check-prefix=CFI-NVT-NO-RV --check-prefix=ITANIUM-TYPEMETADATA --check-prefix=ITANIUM-HIDDEN --check-prefix=ITANIUM-MD --check-prefix=TT-ITANIUM-HIDDEN --check-prefix=NDIAG %s +// RUN: %clang_cc1 -flto -flto-unit -triple x86_64-unknown-linux -fvisibility=hidden -fsanitize=cfi-vcall -emit-llvm -o - %s | FileCheck --check-prefix=CFI --check-prefix=CFI-NVT-NO-RV --check-prefix=ITANIUM-TYPEMETADATA --check-prefix=ITANIUM-HIDDEN --check-prefix=ITANIUM-MD --check-prefix=TT-ITANIUM-HIDDEN --check-prefix=ITANIUM-MD-DIAG --check-prefix=ITANIUM-DIAG --check-prefix=DIAG --check-prefix=DIAG-ABORT %s +// RUN: %clang_cc1 -flto -flto-unit -triple x86_64-unknown-linux -fvisibility=hidden -fsanitize=cfi-vcall -fsanitize-recover=cfi-vcall -emit-llvm -o - %s | FileCheck --check-prefix=CFI --check-prefix=CFI-NVT-NO-RV --check-prefix=ITANIUM-TYPEMETADATA --check-prefix=ITANIUM-HIDDEN --check-prefix=ITANIUM-MD --check-prefix=TT-ITANIUM-HIDDEN --check-prefix=ITANIUM-MD-DIAG --check-prefix=ITANIUM-DIAG --check-prefix=DIAG --check-prefix=DIAG-RECOVER %s +// RUN: %clang_cc1 -flto -flto-unit -triple x86_64-pc-windows-msvc -fsanitize=cfi-vcall -fs
[clang] [Clang] Emit type metadata on vtables when IRPGO option is on. (PR #70841)
https://github.com/minglotus-6 edited https://github.com/llvm/llvm-project/pull/70841 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Emit type metadata on vtables when IRPGO option is on. (PR #70841)
https://github.com/minglotus-6 edited https://github.com/llvm/llvm-project/pull/70841 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [compiler-rt] [IRPGO][ValueProfile] Instrument virtual table address that could be used to do virtual table address comparision for indirect-call-promotion. (PR #66825)
@@ -43,7 +45,8 @@ int main(int argc, const char *argv[]) { uint64_t bufsize = __llvm_profile_get_size_for_buffer_internal( __llvm_profile_begin_data(), __llvm_profile_end_data(), __llvm_profile_begin_counters(), __llvm_profile_end_counters(), - __llvm_profile_begin_names(), __llvm_profile_end_names()); + __llvm_profile_begin_names(), __llvm_profile_end_names(), NULL, NULL, minglotus-6 wrote: Undo the change in this test file (since the previous change of function signature of `__llvm_profile_get_size_for_buffer_internal` is un-done here) https://github.com/llvm/llvm-project/pull/66825 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [compiler-rt] [IRPGO][ValueProfile] Instrument virtual table address that could be used to do virtual table address comparision for indirect-call-promotion. (PR #66825)
@@ -1,7 +1,7 @@ RUN: %clang_pgogen -O2 -fuse-ld=bfd -mllvm -disable-vp=false -mllvm -vp-static-alloc=true -DSTRESS=1 -o %t.ir.warn %S/../Inputs/instrprof-value-prof-real.c RUN: env LLVM_PROFILE_FILE=%t.ir.profraw LLVM_VP_MAX_NUM_VALS_PER_SITE=255 %run %t.ir.warn 2>&1 |FileCheck --check-prefix=WARNING %s # Test that enough static counters have been allocated -RUN: env LLVM_PROFILE_FILE=%t.ir.profraw LLVM_VP_MAX_NUM_VALS_PER_SITE=130 %run %t.ir.warn 2>&1 |FileCheck --check-prefix=NOWARNING --allow-empty %s +RUN: env LLVM_PROFILE_FILE=%t.ir.profraw LLVM_VP_MAX_NUM_VALS_PER_SITE=80 %run %t.ir.warn 2>&1 |FileCheck --check-prefix=NOWARNING --allow-empty %s minglotus-6 wrote: Sorry I missed this. I'll take a look and reply. https://github.com/llvm/llvm-project/pull/66825 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[compiler-rt] [llvm] [clang] [IRPGO][ValueProfile] Instrument virtual table address that could be used to do virtual table address comparision for indirect-call-promotion. (PR #66825)
https://github.com/minglotus-6 edited https://github.com/llvm/llvm-project/pull/66825 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Emit type metadata on vtables when IRPGO instrumentation option is on. (PR #70841)
https://github.com/minglotus-6 edited https://github.com/llvm/llvm-project/pull/70841 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Emit type metadata on vtables when IRPGO instrumentation option is on. (PR #70841)
https://github.com/minglotus-6 edited https://github.com/llvm/llvm-project/pull/70841 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Emit type metadata on vtables when IRPGO instrumentation option is on. (PR #70841)
https://github.com/minglotus-6 updated https://github.com/llvm/llvm-project/pull/70841 >From 99abebc88c09346bd4a70bbf39df13b249116c09 Mon Sep 17 00:00:00 2001 From: Mingming Liu Date: Tue, 31 Oct 2023 01:17:03 -0700 Subject: [PATCH 1/3] [Clang]Emit type metadata when -fprofile-generate is on --- clang/lib/CodeGen/CGVTables.cpp | 2 +- clang/lib/CodeGen/MicrosoftCXXABI.cpp | 3 +- clang/test/CodeGenCXX/type-metadata.cpp | 150 3 files changed, 80 insertions(+), 75 deletions(-) diff --git a/clang/lib/CodeGen/CGVTables.cpp b/clang/lib/CodeGen/CGVTables.cpp index 25e4b1c27932026..895e160dfa8a536 100644 --- a/clang/lib/CodeGen/CGVTables.cpp +++ b/clang/lib/CodeGen/CGVTables.cpp @@ -1312,7 +1312,7 @@ llvm::GlobalObject::VCallVisibility CodeGenModule::GetVCallVisibilityLevel( void CodeGenModule::EmitVTableTypeMetadata(const CXXRecordDecl *RD, llvm::GlobalVariable *VTable, const VTableLayout &VTLayout) { - if (!getCodeGenOpts().LTOUnit) + if (!getCodeGenOpts().LTOUnit && !getCodeGenOpts().hasProfileIRInstr()) return; CharUnits ComponentWidth = GetTargetTypeStoreSize(getVTableComponentType()); diff --git a/clang/lib/CodeGen/MicrosoftCXXABI.cpp b/clang/lib/CodeGen/MicrosoftCXXABI.cpp index d623f8f63ae56c4..740c0b6cb1aac04 100644 --- a/clang/lib/CodeGen/MicrosoftCXXABI.cpp +++ b/clang/lib/CodeGen/MicrosoftCXXABI.cpp @@ -1673,7 +1673,8 @@ void MicrosoftCXXABI::EmitDestructorCall(CodeGenFunction &CGF, void MicrosoftCXXABI::emitVTableTypeMetadata(const VPtrInfo &Info, const CXXRecordDecl *RD, llvm::GlobalVariable *VTable) { - if (!CGM.getCodeGenOpts().LTOUnit) + if (!CGM.getCodeGenOpts().LTOUnit && + !CGM.getCodeGenOpts().hasProfileIRInstr()) return; // TODO: Should VirtualFunctionElimination also be supported here? diff --git a/clang/test/CodeGenCXX/type-metadata.cpp b/clang/test/CodeGenCXX/type-metadata.cpp index 3f8b54af801f8d6..637ec6e6c326ab6 100644 --- a/clang/test/CodeGenCXX/type-metadata.cpp +++ b/clang/test/CodeGenCXX/type-metadata.cpp @@ -1,113 +1,117 @@ // Tests for the cfi-vcall feature: -// RUN: %clang_cc1 -flto -flto-unit -triple x86_64-unknown-linux -fvisibility=hidden -fsanitize=cfi-vcall -fsanitize-trap=cfi-vcall -emit-llvm -o - %s | FileCheck --check-prefix=CFI --check-prefix=CFI-NVT-NO-RV --check-prefix=ITANIUM --check-prefix=ITANIUM-MD --check-prefix=TT-ITANIUM-HIDDEN --check-prefix=NDIAG %s -// RUN: %clang_cc1 -flto -flto-unit -triple x86_64-unknown-linux -fvisibility=hidden -fsanitize=cfi-vcall -emit-llvm -o - %s | FileCheck --check-prefix=CFI --check-prefix=CFI-NVT-NO-RV --check-prefix=ITANIUM --check-prefix=ITANIUM-MD --check-prefix=TT-ITANIUM-HIDDEN --check-prefix=ITANIUM-MD-DIAG --check-prefix=ITANIUM-DIAG --check-prefix=DIAG --check-prefix=DIAG-ABORT %s -// RUN: %clang_cc1 -flto -flto-unit -triple x86_64-unknown-linux -fvisibility=hidden -fsanitize=cfi-vcall -fsanitize-recover=cfi-vcall -emit-llvm -o - %s | FileCheck --check-prefix=CFI --check-prefix=CFI-NVT-NO-RV --check-prefix=ITANIUM --check-prefix=ITANIUM-MD --check-prefix=TT-ITANIUM-HIDDEN --check-prefix=ITANIUM-MD-DIAG --check-prefix=ITANIUM-DIAG --check-prefix=DIAG --check-prefix=DIAG-RECOVER %s -// RUN: %clang_cc1 -flto -flto-unit -triple x86_64-pc-windows-msvc -fsanitize=cfi-vcall -fsanitize-trap=cfi-vcall -emit-llvm -o - %s | FileCheck --check-prefix=CFI --check-prefix=CFI-NVT-NO-RV --check-prefix=MS --check-prefix=TT-MS --check-prefix=NDIAG %s +// RUN: %clang_cc1 -flto -flto-unit -triple x86_64-unknown-linux -fvisibility=hidden -fsanitize=cfi-vcall -fsanitize-trap=cfi-vcall -emit-llvm -o - %s | FileCheck --check-prefix=CFI --check-prefix=CFI-NVT-NO-RV --check-prefix=ITANIUM-TYPEMETADATA --check-prefix=ITANIUM-HIDDEN --check-prefix=ITANIUM-MD --check-prefix=TT-ITANIUM-HIDDEN --check-prefix=NDIAG %s +// RUN: %clang_cc1 -flto -flto-unit -triple x86_64-unknown-linux -fvisibility=hidden -fsanitize=cfi-vcall -emit-llvm -o - %s | FileCheck --check-prefix=CFI --check-prefix=CFI-NVT-NO-RV --check-prefix=ITANIUM-TYPEMETADATA --check-prefix=ITANIUM-HIDDEN --check-prefix=ITANIUM-MD --check-prefix=TT-ITANIUM-HIDDEN --check-prefix=ITANIUM-MD-DIAG --check-prefix=ITANIUM-DIAG --check-prefix=DIAG --check-prefix=DIAG-ABORT %s +// RUN: %clang_cc1 -flto -flto-unit -triple x86_64-unknown-linux -fvisibility=hidden -fsanitize=cfi-vcall -fsanitize-recover=cfi-vcall -emit-llvm -o - %s | FileCheck --check-prefix=CFI --check-prefix=CFI-NVT-NO-RV --check-prefix=ITANIUM-TYPEMETADATA --check-prefix=ITANIUM-HIDDEN --check-prefix=ITANIUM-MD --check-prefix=TT-ITANIUM-HIDDEN --check-prefix=ITANIUM-MD-DIAG --check-prefix=ITANIUM-DIAG --check-prefix=DIAG --check-prefix=DIAG-RECOVER %s +// RUN: %clang_cc1 -flto -flto-unit -triple x86_64-pc-windows-msvc -fsanitize=cfi-vcall -fs
[clang] [Clang] Emit type metadata on vtables when IRPGO instrumentation option is on. (PR #70841)
https://github.com/minglotus-6 ready_for_review https://github.com/llvm/llvm-project/pull/70841 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Emit type metadata on vtables when IRPGO instrumentation option is on. (PR #70841)
@@ -0,0 +1,145 @@ +// Test that type metadata are emitted with -fprofile-generate +// +// RUN: %clang -fprofile-generate -fno-lto -target x86_64-unknown-linux -emit-llvm -S %s -o - | FileCheck %s --check-prefix=ITANIUM +// RUN: %clang -fprofile-generate -fno-lto -target x86_64-pc-windows-msvc -emit-llvm -S %s -o - | FileCheck %s --check-prefix=MS minglotus-6 wrote: This test is forked from https://github.com/llvm/llvm-project/blob/main/clang/test/CodeGenCXX/type-metadata.cpp. It's currently put under directory clang/test/Driver since `-fprofile-generate` is a driver option. I'm wondering if it's more idiomatic to create the corresponding clang compiler options for `-fprofile-generate` for what I'm trying to do here? That way I could update `clang/test/CodeGenCXX/type-metadata.cpp` for test coverage. https://github.com/llvm/llvm-project/pull/70841 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[llvm] [compiler-rt] [clang] [IRPGO][ValueProfile] Instrument virtual table address that could be used to do virtual table address comparision for indirect-call-promotion. (PR #66825)
@@ -1,7 +1,7 @@ RUN: %clang_pgogen -O2 -fuse-ld=bfd -mllvm -disable-vp=false -mllvm -vp-static-alloc=true -DSTRESS=1 -o %t.ir.warn %S/../Inputs/instrprof-value-prof-real.c RUN: env LLVM_PROFILE_FILE=%t.ir.profraw LLVM_VP_MAX_NUM_VALS_PER_SITE=255 %run %t.ir.warn 2>&1 |FileCheck --check-prefix=WARNING %s # Test that enough static counters have been allocated -RUN: env LLVM_PROFILE_FILE=%t.ir.profraw LLVM_VP_MAX_NUM_VALS_PER_SITE=130 %run %t.ir.warn 2>&1 |FileCheck --check-prefix=NOWARNING --allow-empty %s +RUN: env LLVM_PROFILE_FILE=%t.ir.profraw LLVM_VP_MAX_NUM_VALS_PER_SITE=80 %run %t.ir.warn 2>&1 |FileCheck --check-prefix=NOWARNING --allow-empty %s minglotus-6 wrote: The TL,DR is this change is only needed when `enable-vtable-value-profiling` is true. Undo this change since `enable-vtable-value-profiling` is false by default. This test expects errors like "LLVM Profile Warning: Unable to track new values: Running out of static counters. Consider using option -mllvm -vp-counters-per-site= to allocate more value profile counters at compile time." when `LLVM_VP_MAX_NUM_VALS_PER_SITE` (the max number of allocated counters per instrumented site) is 255, and expects no such warnings when the env var is set to 130. - The number of indirect callees in the tested c file is [approximately 255](https://github.com/llvm/llvm-project/blob/ec350ad418a24f70c88758259c774a1e11c06d74/compiler-rt/test/profile/Inputs/instrprof-value-prof-real.c#L75-L328). With `vp-static-alloc=true`, the number of counters is the `min(10, the number of instrumented sites in the function IR * the value of vp-counters-per-site`, computed at compile time [here](https://github.com/llvm/llvm-project/blob/ec350ad418a24f70c88758259c774a1e11c06d74/llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp#L1402-L1412); and `LLVM_VP_MAX_NUM_VALS_PER_SITE` specifies the max number of collected values per site at runtime; when a site runs out of nodes, the coldest ones are [evicted first](https://github.com/llvm/llvm-project/blob/ec350ad418a24f70c88758259c774a1e11c06d74/compiler-rt/lib/profile/InstrProfilingValue.c#L178-L215). Without the refinement to insert type intrinsics and pick instrumentation points precisely, `enable-vtable-value-profiling` means false positives takes up some counters and thereby the need to reduce `LLVM_VP_MAX_NUM_VALS_PER_SITE` to make the test pass. https://github.com/llvm/llvm-project/pull/66825 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Emit type metadata on vtables when IRPGO instrumentation option is on. (PR #70841)
https://github.com/minglotus-6 updated https://github.com/llvm/llvm-project/pull/70841 >From 99abebc88c09346bd4a70bbf39df13b249116c09 Mon Sep 17 00:00:00 2001 From: Mingming Liu Date: Tue, 31 Oct 2023 01:17:03 -0700 Subject: [PATCH 1/4] [Clang]Emit type metadata when -fprofile-generate is on --- clang/lib/CodeGen/CGVTables.cpp | 2 +- clang/lib/CodeGen/MicrosoftCXXABI.cpp | 3 +- clang/test/CodeGenCXX/type-metadata.cpp | 150 3 files changed, 80 insertions(+), 75 deletions(-) diff --git a/clang/lib/CodeGen/CGVTables.cpp b/clang/lib/CodeGen/CGVTables.cpp index 25e4b1c27932026..895e160dfa8a536 100644 --- a/clang/lib/CodeGen/CGVTables.cpp +++ b/clang/lib/CodeGen/CGVTables.cpp @@ -1312,7 +1312,7 @@ llvm::GlobalObject::VCallVisibility CodeGenModule::GetVCallVisibilityLevel( void CodeGenModule::EmitVTableTypeMetadata(const CXXRecordDecl *RD, llvm::GlobalVariable *VTable, const VTableLayout &VTLayout) { - if (!getCodeGenOpts().LTOUnit) + if (!getCodeGenOpts().LTOUnit && !getCodeGenOpts().hasProfileIRInstr()) return; CharUnits ComponentWidth = GetTargetTypeStoreSize(getVTableComponentType()); diff --git a/clang/lib/CodeGen/MicrosoftCXXABI.cpp b/clang/lib/CodeGen/MicrosoftCXXABI.cpp index d623f8f63ae56c4..740c0b6cb1aac04 100644 --- a/clang/lib/CodeGen/MicrosoftCXXABI.cpp +++ b/clang/lib/CodeGen/MicrosoftCXXABI.cpp @@ -1673,7 +1673,8 @@ void MicrosoftCXXABI::EmitDestructorCall(CodeGenFunction &CGF, void MicrosoftCXXABI::emitVTableTypeMetadata(const VPtrInfo &Info, const CXXRecordDecl *RD, llvm::GlobalVariable *VTable) { - if (!CGM.getCodeGenOpts().LTOUnit) + if (!CGM.getCodeGenOpts().LTOUnit && + !CGM.getCodeGenOpts().hasProfileIRInstr()) return; // TODO: Should VirtualFunctionElimination also be supported here? diff --git a/clang/test/CodeGenCXX/type-metadata.cpp b/clang/test/CodeGenCXX/type-metadata.cpp index 3f8b54af801f8d6..637ec6e6c326ab6 100644 --- a/clang/test/CodeGenCXX/type-metadata.cpp +++ b/clang/test/CodeGenCXX/type-metadata.cpp @@ -1,113 +1,117 @@ // Tests for the cfi-vcall feature: -// RUN: %clang_cc1 -flto -flto-unit -triple x86_64-unknown-linux -fvisibility=hidden -fsanitize=cfi-vcall -fsanitize-trap=cfi-vcall -emit-llvm -o - %s | FileCheck --check-prefix=CFI --check-prefix=CFI-NVT-NO-RV --check-prefix=ITANIUM --check-prefix=ITANIUM-MD --check-prefix=TT-ITANIUM-HIDDEN --check-prefix=NDIAG %s -// RUN: %clang_cc1 -flto -flto-unit -triple x86_64-unknown-linux -fvisibility=hidden -fsanitize=cfi-vcall -emit-llvm -o - %s | FileCheck --check-prefix=CFI --check-prefix=CFI-NVT-NO-RV --check-prefix=ITANIUM --check-prefix=ITANIUM-MD --check-prefix=TT-ITANIUM-HIDDEN --check-prefix=ITANIUM-MD-DIAG --check-prefix=ITANIUM-DIAG --check-prefix=DIAG --check-prefix=DIAG-ABORT %s -// RUN: %clang_cc1 -flto -flto-unit -triple x86_64-unknown-linux -fvisibility=hidden -fsanitize=cfi-vcall -fsanitize-recover=cfi-vcall -emit-llvm -o - %s | FileCheck --check-prefix=CFI --check-prefix=CFI-NVT-NO-RV --check-prefix=ITANIUM --check-prefix=ITANIUM-MD --check-prefix=TT-ITANIUM-HIDDEN --check-prefix=ITANIUM-MD-DIAG --check-prefix=ITANIUM-DIAG --check-prefix=DIAG --check-prefix=DIAG-RECOVER %s -// RUN: %clang_cc1 -flto -flto-unit -triple x86_64-pc-windows-msvc -fsanitize=cfi-vcall -fsanitize-trap=cfi-vcall -emit-llvm -o - %s | FileCheck --check-prefix=CFI --check-prefix=CFI-NVT-NO-RV --check-prefix=MS --check-prefix=TT-MS --check-prefix=NDIAG %s +// RUN: %clang_cc1 -flto -flto-unit -triple x86_64-unknown-linux -fvisibility=hidden -fsanitize=cfi-vcall -fsanitize-trap=cfi-vcall -emit-llvm -o - %s | FileCheck --check-prefix=CFI --check-prefix=CFI-NVT-NO-RV --check-prefix=ITANIUM-TYPEMETADATA --check-prefix=ITANIUM-HIDDEN --check-prefix=ITANIUM-MD --check-prefix=TT-ITANIUM-HIDDEN --check-prefix=NDIAG %s +// RUN: %clang_cc1 -flto -flto-unit -triple x86_64-unknown-linux -fvisibility=hidden -fsanitize=cfi-vcall -emit-llvm -o - %s | FileCheck --check-prefix=CFI --check-prefix=CFI-NVT-NO-RV --check-prefix=ITANIUM-TYPEMETADATA --check-prefix=ITANIUM-HIDDEN --check-prefix=ITANIUM-MD --check-prefix=TT-ITANIUM-HIDDEN --check-prefix=ITANIUM-MD-DIAG --check-prefix=ITANIUM-DIAG --check-prefix=DIAG --check-prefix=DIAG-ABORT %s +// RUN: %clang_cc1 -flto -flto-unit -triple x86_64-unknown-linux -fvisibility=hidden -fsanitize=cfi-vcall -fsanitize-recover=cfi-vcall -emit-llvm -o - %s | FileCheck --check-prefix=CFI --check-prefix=CFI-NVT-NO-RV --check-prefix=ITANIUM-TYPEMETADATA --check-prefix=ITANIUM-HIDDEN --check-prefix=ITANIUM-MD --check-prefix=TT-ITANIUM-HIDDEN --check-prefix=ITANIUM-MD-DIAG --check-prefix=ITANIUM-DIAG --check-prefix=DIAG --check-prefix=DIAG-RECOVER %s +// RUN: %clang_cc1 -flto -flto-unit -triple x86_64-pc-windows-msvc -fsanitize=cfi-vcall -fs
[clang] [Clang] Emit type metadata on vtables when IRPGO instrumentation option is on. (PR #70841)
https://github.com/minglotus-6 updated https://github.com/llvm/llvm-project/pull/70841 >From 99abebc88c09346bd4a70bbf39df13b249116c09 Mon Sep 17 00:00:00 2001 From: Mingming Liu Date: Tue, 31 Oct 2023 01:17:03 -0700 Subject: [PATCH 1/5] [Clang]Emit type metadata when -fprofile-generate is on --- clang/lib/CodeGen/CGVTables.cpp | 2 +- clang/lib/CodeGen/MicrosoftCXXABI.cpp | 3 +- clang/test/CodeGenCXX/type-metadata.cpp | 150 3 files changed, 80 insertions(+), 75 deletions(-) diff --git a/clang/lib/CodeGen/CGVTables.cpp b/clang/lib/CodeGen/CGVTables.cpp index 25e4b1c27932026..895e160dfa8a536 100644 --- a/clang/lib/CodeGen/CGVTables.cpp +++ b/clang/lib/CodeGen/CGVTables.cpp @@ -1312,7 +1312,7 @@ llvm::GlobalObject::VCallVisibility CodeGenModule::GetVCallVisibilityLevel( void CodeGenModule::EmitVTableTypeMetadata(const CXXRecordDecl *RD, llvm::GlobalVariable *VTable, const VTableLayout &VTLayout) { - if (!getCodeGenOpts().LTOUnit) + if (!getCodeGenOpts().LTOUnit && !getCodeGenOpts().hasProfileIRInstr()) return; CharUnits ComponentWidth = GetTargetTypeStoreSize(getVTableComponentType()); diff --git a/clang/lib/CodeGen/MicrosoftCXXABI.cpp b/clang/lib/CodeGen/MicrosoftCXXABI.cpp index d623f8f63ae56c4..740c0b6cb1aac04 100644 --- a/clang/lib/CodeGen/MicrosoftCXXABI.cpp +++ b/clang/lib/CodeGen/MicrosoftCXXABI.cpp @@ -1673,7 +1673,8 @@ void MicrosoftCXXABI::EmitDestructorCall(CodeGenFunction &CGF, void MicrosoftCXXABI::emitVTableTypeMetadata(const VPtrInfo &Info, const CXXRecordDecl *RD, llvm::GlobalVariable *VTable) { - if (!CGM.getCodeGenOpts().LTOUnit) + if (!CGM.getCodeGenOpts().LTOUnit && + !CGM.getCodeGenOpts().hasProfileIRInstr()) return; // TODO: Should VirtualFunctionElimination also be supported here? diff --git a/clang/test/CodeGenCXX/type-metadata.cpp b/clang/test/CodeGenCXX/type-metadata.cpp index 3f8b54af801f8d6..637ec6e6c326ab6 100644 --- a/clang/test/CodeGenCXX/type-metadata.cpp +++ b/clang/test/CodeGenCXX/type-metadata.cpp @@ -1,113 +1,117 @@ // Tests for the cfi-vcall feature: -// RUN: %clang_cc1 -flto -flto-unit -triple x86_64-unknown-linux -fvisibility=hidden -fsanitize=cfi-vcall -fsanitize-trap=cfi-vcall -emit-llvm -o - %s | FileCheck --check-prefix=CFI --check-prefix=CFI-NVT-NO-RV --check-prefix=ITANIUM --check-prefix=ITANIUM-MD --check-prefix=TT-ITANIUM-HIDDEN --check-prefix=NDIAG %s -// RUN: %clang_cc1 -flto -flto-unit -triple x86_64-unknown-linux -fvisibility=hidden -fsanitize=cfi-vcall -emit-llvm -o - %s | FileCheck --check-prefix=CFI --check-prefix=CFI-NVT-NO-RV --check-prefix=ITANIUM --check-prefix=ITANIUM-MD --check-prefix=TT-ITANIUM-HIDDEN --check-prefix=ITANIUM-MD-DIAG --check-prefix=ITANIUM-DIAG --check-prefix=DIAG --check-prefix=DIAG-ABORT %s -// RUN: %clang_cc1 -flto -flto-unit -triple x86_64-unknown-linux -fvisibility=hidden -fsanitize=cfi-vcall -fsanitize-recover=cfi-vcall -emit-llvm -o - %s | FileCheck --check-prefix=CFI --check-prefix=CFI-NVT-NO-RV --check-prefix=ITANIUM --check-prefix=ITANIUM-MD --check-prefix=TT-ITANIUM-HIDDEN --check-prefix=ITANIUM-MD-DIAG --check-prefix=ITANIUM-DIAG --check-prefix=DIAG --check-prefix=DIAG-RECOVER %s -// RUN: %clang_cc1 -flto -flto-unit -triple x86_64-pc-windows-msvc -fsanitize=cfi-vcall -fsanitize-trap=cfi-vcall -emit-llvm -o - %s | FileCheck --check-prefix=CFI --check-prefix=CFI-NVT-NO-RV --check-prefix=MS --check-prefix=TT-MS --check-prefix=NDIAG %s +// RUN: %clang_cc1 -flto -flto-unit -triple x86_64-unknown-linux -fvisibility=hidden -fsanitize=cfi-vcall -fsanitize-trap=cfi-vcall -emit-llvm -o - %s | FileCheck --check-prefix=CFI --check-prefix=CFI-NVT-NO-RV --check-prefix=ITANIUM-TYPEMETADATA --check-prefix=ITANIUM-HIDDEN --check-prefix=ITANIUM-MD --check-prefix=TT-ITANIUM-HIDDEN --check-prefix=NDIAG %s +// RUN: %clang_cc1 -flto -flto-unit -triple x86_64-unknown-linux -fvisibility=hidden -fsanitize=cfi-vcall -emit-llvm -o - %s | FileCheck --check-prefix=CFI --check-prefix=CFI-NVT-NO-RV --check-prefix=ITANIUM-TYPEMETADATA --check-prefix=ITANIUM-HIDDEN --check-prefix=ITANIUM-MD --check-prefix=TT-ITANIUM-HIDDEN --check-prefix=ITANIUM-MD-DIAG --check-prefix=ITANIUM-DIAG --check-prefix=DIAG --check-prefix=DIAG-ABORT %s +// RUN: %clang_cc1 -flto -flto-unit -triple x86_64-unknown-linux -fvisibility=hidden -fsanitize=cfi-vcall -fsanitize-recover=cfi-vcall -emit-llvm -o - %s | FileCheck --check-prefix=CFI --check-prefix=CFI-NVT-NO-RV --check-prefix=ITANIUM-TYPEMETADATA --check-prefix=ITANIUM-HIDDEN --check-prefix=ITANIUM-MD --check-prefix=TT-ITANIUM-HIDDEN --check-prefix=ITANIUM-MD-DIAG --check-prefix=ITANIUM-DIAG --check-prefix=DIAG --check-prefix=DIAG-RECOVER %s +// RUN: %clang_cc1 -flto -flto-unit -triple x86_64-pc-windows-msvc -fsanitize=cfi-vcall -fs
[clang] [Clang] Emit type metadata on vtables when IRPGO instrumentation option is on. (PR #70841)
@@ -0,0 +1,145 @@ +// Test that type metadata are emitted with -fprofile-generate +// +// RUN: %clang -fprofile-generate -fno-lto -target x86_64-unknown-linux -emit-llvm -S %s -o - | FileCheck %s --check-prefix=ITANIUM +// RUN: %clang -fprofile-generate -fno-lto -target x86_64-pc-windows-msvc -emit-llvm -S %s -o - | FileCheck %s --check-prefix=MS minglotus-6 wrote: thanks! I updated the existing test (and removed the forked one) by only generating `!type` annotations iff clang cc1 option `-fprofile-instrument` is set to `llvm` (the FDO build). CS-FDO and clang instrumentation sets this cc1 option differently, and it's intentional for this PR not to emit `!type` in these two settings mainly because there is no usage. A longer story from some interesting findings when I tried to figured out how clang options work for the use case in this pr. Currently, clang driver accepts two options for FDO 1. `-fprofile-generate` (a [`Flag`](https://github.com/llvm/llvm-project/blob/c14e086b23929b0ac7092c45349a4f9749eda39c/clang/include/clang/Driver/Options.td#L1653) option that doesn't accept value according to https://clang.llvm.org/docs/InternalsManual.html#adding-new-command-line-option 2. `-fprofile-generate=` (a [`Joined`](https://github.com/llvm/llvm-project/blob/c14e086b23929b0ac7092c45349a4f9749eda39c/clang/include/clang/Driver/Options.td#L1656) option that accepts the value I initially add [`CC1Option`](https://github.com/llvm/llvm-project/blob/c14e086b23929b0ac7092c45349a4f9749eda39c/clang/include/clang/Driver/Options.td#L64) to both (and added corresponding codegen options); and during this process found `-fprofile-instrument=llvm` is true if clang user either specified `-fprofile-generate` or `-fprofile-generate=path/to` (but not CS-FDO or clang instrumentation FDO), so decided to just rely on this. https://github.com/llvm/llvm-project/pull/70841 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Emit type metadata on vtables when IRPGO instrumentation option is on. (PR #70841)
https://github.com/minglotus-6 edited https://github.com/llvm/llvm-project/pull/70841 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Emit type metadata on vtables when IRPGO instrumentation option is on. (PR #70841)
https://github.com/minglotus-6 edited https://github.com/llvm/llvm-project/pull/70841 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Emit type metadata on vtables when IRPGO instrumentation option is on. (PR #70841)
https://github.com/minglotus-6 edited https://github.com/llvm/llvm-project/pull/70841 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Emit type metadata on vtables when IRPGO instrumentation option is on. (PR #70841)
@@ -1,98 +1,116 @@ + // Tests for the cfi-vcall feature: -// RUN: %clang_cc1 -flto -flto-unit -triple x86_64-unknown-linux -fvisibility=hidden -fsanitize=cfi-vcall -fsanitize-trap=cfi-vcall -emit-llvm -o - %s | FileCheck --check-prefix=CFI --check-prefix=CFI-NVT-NO-RV --check-prefix=ITANIUM --check-prefix=ITANIUM-MD --check-prefix=TT-ITANIUM-HIDDEN --check-prefix=NDIAG %s -// RUN: %clang_cc1 -flto -flto-unit -triple x86_64-unknown-linux -fvisibility=hidden -fsanitize=cfi-vcall -emit-llvm -o - %s | FileCheck --check-prefix=CFI --check-prefix=CFI-NVT-NO-RV --check-prefix=ITANIUM --check-prefix=ITANIUM-MD --check-prefix=TT-ITANIUM-HIDDEN --check-prefix=ITANIUM-MD-DIAG --check-prefix=ITANIUM-DIAG --check-prefix=DIAG --check-prefix=DIAG-ABORT %s -// RUN: %clang_cc1 -flto -flto-unit -triple x86_64-unknown-linux -fvisibility=hidden -fsanitize=cfi-vcall -fsanitize-recover=cfi-vcall -emit-llvm -o - %s | FileCheck --check-prefix=CFI --check-prefix=CFI-NVT-NO-RV --check-prefix=ITANIUM --check-prefix=ITANIUM-MD --check-prefix=TT-ITANIUM-HIDDEN --check-prefix=ITANIUM-MD-DIAG --check-prefix=ITANIUM-DIAG --check-prefix=DIAG --check-prefix=DIAG-RECOVER %s -// RUN: %clang_cc1 -flto -flto-unit -triple x86_64-pc-windows-msvc -fsanitize=cfi-vcall -fsanitize-trap=cfi-vcall -emit-llvm -o - %s | FileCheck --check-prefix=CFI --check-prefix=CFI-NVT-NO-RV --check-prefix=MS --check-prefix=TT-MS --check-prefix=NDIAG %s +// RUN: %clang_cc1 -flto -flto-unit -triple x86_64-unknown-linux -fvisibility=hidden -fsanitize=cfi-vcall -fsanitize-trap=cfi-vcall -emit-llvm -o - %s | FileCheck --check-prefix=CFI --check-prefix=CFI-NVT-NO-RV --check-prefix=ITANIUM --check-prefix=ITANIUM-TYPEMETADATA --check-prefix=ITANIUM-MD --check-prefix=TT-ITANIUM-HIDDEN --check-prefix=NDIAG %s +// RUN: %clang_cc1 -flto -flto-unit -triple x86_64-unknown-linux -fvisibility=hidden -fsanitize=cfi-vcall -emit-llvm -o - %s | FileCheck --check-prefix=CFI --check-prefix=CFI-NVT-NO-RV --check-prefix=ITANIUM --check-prefix=ITANIUM-TYPEMETADATA --check-prefix=ITANIUM-MD --check-prefix=TT-ITANIUM-HIDDEN --check-prefix=ITANIUM-MD-DIAG --check-prefix=ITANIUM-DIAG --check-prefix=DIAG --check-prefix=DIAG-ABORT %s +// RUN: %clang_cc1 -flto -flto-unit -triple x86_64-unknown-linux -fvisibility=hidden -fsanitize=cfi-vcall -fsanitize-recover=cfi-vcall -emit-llvm -o - %s | FileCheck --check-prefix=CFI --check-prefix=CFI-NVT-NO-RV --check-prefix=ITANIUM --check-prefix=ITANIUM-TYPEMETADATA --check-prefix=ITANIUM-MD --check-prefix=TT-ITANIUM-HIDDEN --check-prefix=ITANIUM-MD-DIAG --check-prefix=ITANIUM-DIAG --check-prefix=DIAG --check-prefix=DIAG-RECOVER %s +// RUN: %clang_cc1 -flto -flto-unit -triple x86_64-pc-windows-msvc -fsanitize=cfi-vcall -fsanitize-trap=cfi-vcall -emit-llvm -o - %s | FileCheck --check-prefix=CFI --check-prefix=CFI-NVT-NO-RV --check-prefix=MS --check-prefix=MS-TYPEMETADATA --check-prefix=TT-MS --check-prefix=NDIAG %s // Tests for the whole-program-vtables feature: -// RUN: %clang_cc1 -flto -flto-unit -triple x86_64-unknown-linux -fvisibility=hidden -fwhole-program-vtables -emit-llvm -o - %s | FileCheck --check-prefix=VTABLE-OPT --check-prefix=ITANIUM --check-prefix=ITANIUM-MD --check-prefix=TT-ITANIUM-HIDDEN %s +// RUN: %clang_cc1 -flto -flto-unit -triple x86_64-unknown-linux -fvisibility=hidden -fwhole-program-vtables -emit-llvm -o - %s | FileCheck --check-prefix=VTABLE-OPT --check-prefix=ITANIUM --check-prefix=ITANIUM-TYPEMETADATA --check-prefix=ITANIUM-MD --check-prefix=TT-ITANIUM-HIDDEN %s // RUN: %clang_cc1 -flto -flto-unit -triple x86_64-unknown-linux -fwhole-program-vtables -emit-llvm -o - %s | FileCheck --check-prefix=VTABLE-OPT --check-prefix=ITANIUM-DEFAULTVIS --check-prefix=TT-ITANIUM-DEFAULT %s // RUN: %clang_cc1 -O2 -flto -flto-unit -triple x86_64-unknown-linux -fwhole-program-vtables -emit-llvm -o - %s | FileCheck --check-prefix=ITANIUM-OPT --check-prefix=ITANIUM-OPT-LAYOUT %s -// RUN: %clang_cc1 -flto -flto-unit -triple x86_64-pc-windows-msvc -fwhole-program-vtables -emit-llvm -o - %s | FileCheck --check-prefix=VTABLE-OPT --check-prefix=MS --check-prefix=TT-MS %s +// RUN: %clang_cc1 -flto -flto-unit -triple x86_64-pc-windows-msvc -fwhole-program-vtables -emit-llvm -o - %s | FileCheck --check-prefix=VTABLE-OPT --check-prefix=MS --check-prefix=MS-TYPEMETADATA --check-prefix=TT-MS %s // Tests for cfi + whole-program-vtables: -// RUN: %clang_cc1 -flto -flto-unit -triple x86_64-unknown-linux -fvisibility=hidden -fsanitize=cfi-vcall -fsanitize-trap=cfi-vcall -fwhole-program-vtables -emit-llvm -o - %s | FileCheck --check-prefix=CFI --check-prefix=CFI-VT --check-prefix=ITANIUM --check-prefix=TC-ITANIUM --check-prefix=ITANIUM-MD %s -// RUN: %clang_cc1 -flto -flto-unit -triple x86_64-pc-windows-msvc -fsanitize=cfi-vcall -fsanitize-trap=cfi-vcall -fwhole-program-vtables -emit-llvm -o - %s | FileCheck --check-prefix=CFI --check-prefix=CFI-VT --check-prefix=MS --check-prefix=T
[clang] [Clang] Emit type metadata on vtables when IRPGO instrumentation option is on. (PR #70841)
@@ -101,14 +119,16 @@ // ITANIUM-OPT-SAME: !type [[EF16:![0-9]+]] // ITANIUM-OPT: @llvm.compiler.used = appending global [1 x ptr] [ptr @_ZTVN5test31EE] -// MS: comdat($"??_7A@@6B@"), !type [[A8:![0-9]+]] -// MS: comdat($"??_7B@@6B0@@"), !type [[B8:![0-9]+]] -// MS: comdat($"??_7B@@6BA@@@"), !type [[A8]] -// MS: comdat($"??_7C@@6B@"), !type [[A8]] -// MS: comdat($"??_7D@?A0x{{[^@]*}}@@6BB@@@"), !type [[B8]], !type [[D8:![0-9]+]] -// MS: comdat($"??_7D@?A0x{{[^@]*}}@@6BA@@@"), !type [[A8]] -// MS: comdat($"??_7FA@?1??foo@@YAXXZ@6B@"), !type [[A8]], !type [[FA8:![0-9]+]] +// MS-TYPEMETADATA: comdat($"??_7A@@6B@"), !type [[A8:![0-9]+]] +// MS-TYPEMETADATA: comdat($"??_7B@@6B0@@"), !type [[B8:![0-9]+]] +// MS-TYPEMETADATA: comdat($"??_7B@@6BA@@@"), !type [[A8]] +// MS-TYPEMETADATA: comdat($"??_7C@@6B@"), !type [[A8]] +// MS-TYPEMETADATA: comdat($"??_7D@?A0x{{[^@]*}}@@6BB@@@"), !type [[B8]], !type [[D8:![0-9]+]] +// MS-TYPEMETADATA: comdat($"??_7D@?A0x{{[^@]*}}@@6BA@@@"), !type [[A8]] +// MS-TYPEMETADATA: comdat($"??_7FA@?1??foo@@YAXXZ@6B@"), !type [[A8]], !type [[FA8:![0-9]+]] +// Test !type doesn't exist in the generated IR. +// NOTYPEMD-NOT: !type minglotus-6 wrote: done. https://github.com/llvm/llvm-project/pull/70841 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Emit type metadata on vtables when IRPGO instrumentation option is on. (PR #70841)
@@ -1312,7 +1312,7 @@ llvm::GlobalObject::VCallVisibility CodeGenModule::GetVCallVisibilityLevel( void CodeGenModule::EmitVTableTypeMetadata(const CXXRecordDecl *RD, llvm::GlobalVariable *VTable, const VTableLayout &VTLayout) { - if (!getCodeGenOpts().LTOUnit) + if (!getCodeGenOpts().LTOUnit && !getCodeGenOpts().hasProfileIRInstr()) minglotus-6 wrote: done. https://github.com/llvm/llvm-project/pull/70841 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Emit type metadata on vtables when IRPGO instrumentation option is on. (PR #70841)
@@ -1,98 +1,116 @@ + // Tests for the cfi-vcall feature: -// RUN: %clang_cc1 -flto -flto-unit -triple x86_64-unknown-linux -fvisibility=hidden -fsanitize=cfi-vcall -fsanitize-trap=cfi-vcall -emit-llvm -o - %s | FileCheck --check-prefix=CFI --check-prefix=CFI-NVT-NO-RV --check-prefix=ITANIUM --check-prefix=ITANIUM-MD --check-prefix=TT-ITANIUM-HIDDEN --check-prefix=NDIAG %s -// RUN: %clang_cc1 -flto -flto-unit -triple x86_64-unknown-linux -fvisibility=hidden -fsanitize=cfi-vcall -emit-llvm -o - %s | FileCheck --check-prefix=CFI --check-prefix=CFI-NVT-NO-RV --check-prefix=ITANIUM --check-prefix=ITANIUM-MD --check-prefix=TT-ITANIUM-HIDDEN --check-prefix=ITANIUM-MD-DIAG --check-prefix=ITANIUM-DIAG --check-prefix=DIAG --check-prefix=DIAG-ABORT %s -// RUN: %clang_cc1 -flto -flto-unit -triple x86_64-unknown-linux -fvisibility=hidden -fsanitize=cfi-vcall -fsanitize-recover=cfi-vcall -emit-llvm -o - %s | FileCheck --check-prefix=CFI --check-prefix=CFI-NVT-NO-RV --check-prefix=ITANIUM --check-prefix=ITANIUM-MD --check-prefix=TT-ITANIUM-HIDDEN --check-prefix=ITANIUM-MD-DIAG --check-prefix=ITANIUM-DIAG --check-prefix=DIAG --check-prefix=DIAG-RECOVER %s -// RUN: %clang_cc1 -flto -flto-unit -triple x86_64-pc-windows-msvc -fsanitize=cfi-vcall -fsanitize-trap=cfi-vcall -emit-llvm -o - %s | FileCheck --check-prefix=CFI --check-prefix=CFI-NVT-NO-RV --check-prefix=MS --check-prefix=TT-MS --check-prefix=NDIAG %s +// RUN: %clang_cc1 -flto -flto-unit -triple x86_64-unknown-linux -fvisibility=hidden -fsanitize=cfi-vcall -fsanitize-trap=cfi-vcall -emit-llvm -o - %s | FileCheck --check-prefix=CFI --check-prefix=CFI-NVT-NO-RV --check-prefix=ITANIUM --check-prefix=ITANIUM-TYPEMETADATA --check-prefix=ITANIUM-MD --check-prefix=TT-ITANIUM-HIDDEN --check-prefix=NDIAG %s +// RUN: %clang_cc1 -flto -flto-unit -triple x86_64-unknown-linux -fvisibility=hidden -fsanitize=cfi-vcall -emit-llvm -o - %s | FileCheck --check-prefix=CFI --check-prefix=CFI-NVT-NO-RV --check-prefix=ITANIUM --check-prefix=ITANIUM-TYPEMETADATA --check-prefix=ITANIUM-MD --check-prefix=TT-ITANIUM-HIDDEN --check-prefix=ITANIUM-MD-DIAG --check-prefix=ITANIUM-DIAG --check-prefix=DIAG --check-prefix=DIAG-ABORT %s +// RUN: %clang_cc1 -flto -flto-unit -triple x86_64-unknown-linux -fvisibility=hidden -fsanitize=cfi-vcall -fsanitize-recover=cfi-vcall -emit-llvm -o - %s | FileCheck --check-prefix=CFI --check-prefix=CFI-NVT-NO-RV --check-prefix=ITANIUM --check-prefix=ITANIUM-TYPEMETADATA --check-prefix=ITANIUM-MD --check-prefix=TT-ITANIUM-HIDDEN --check-prefix=ITANIUM-MD-DIAG --check-prefix=ITANIUM-DIAG --check-prefix=DIAG --check-prefix=DIAG-RECOVER %s +// RUN: %clang_cc1 -flto -flto-unit -triple x86_64-pc-windows-msvc -fsanitize=cfi-vcall -fsanitize-trap=cfi-vcall -emit-llvm -o - %s | FileCheck --check-prefix=CFI --check-prefix=CFI-NVT-NO-RV --check-prefix=MS --check-prefix=MS-TYPEMETADATA --check-prefix=TT-MS --check-prefix=NDIAG %s // Tests for the whole-program-vtables feature: -// RUN: %clang_cc1 -flto -flto-unit -triple x86_64-unknown-linux -fvisibility=hidden -fwhole-program-vtables -emit-llvm -o - %s | FileCheck --check-prefix=VTABLE-OPT --check-prefix=ITANIUM --check-prefix=ITANIUM-MD --check-prefix=TT-ITANIUM-HIDDEN %s +// RUN: %clang_cc1 -flto -flto-unit -triple x86_64-unknown-linux -fvisibility=hidden -fwhole-program-vtables -emit-llvm -o - %s | FileCheck --check-prefix=VTABLE-OPT --check-prefix=ITANIUM --check-prefix=ITANIUM-TYPEMETADATA --check-prefix=ITANIUM-MD --check-prefix=TT-ITANIUM-HIDDEN %s // RUN: %clang_cc1 -flto -flto-unit -triple x86_64-unknown-linux -fwhole-program-vtables -emit-llvm -o - %s | FileCheck --check-prefix=VTABLE-OPT --check-prefix=ITANIUM-DEFAULTVIS --check-prefix=TT-ITANIUM-DEFAULT %s // RUN: %clang_cc1 -O2 -flto -flto-unit -triple x86_64-unknown-linux -fwhole-program-vtables -emit-llvm -o - %s | FileCheck --check-prefix=ITANIUM-OPT --check-prefix=ITANIUM-OPT-LAYOUT %s -// RUN: %clang_cc1 -flto -flto-unit -triple x86_64-pc-windows-msvc -fwhole-program-vtables -emit-llvm -o - %s | FileCheck --check-prefix=VTABLE-OPT --check-prefix=MS --check-prefix=TT-MS %s +// RUN: %clang_cc1 -flto -flto-unit -triple x86_64-pc-windows-msvc -fwhole-program-vtables -emit-llvm -o - %s | FileCheck --check-prefix=VTABLE-OPT --check-prefix=MS --check-prefix=MS-TYPEMETADATA --check-prefix=TT-MS %s // Tests for cfi + whole-program-vtables: -// RUN: %clang_cc1 -flto -flto-unit -triple x86_64-unknown-linux -fvisibility=hidden -fsanitize=cfi-vcall -fsanitize-trap=cfi-vcall -fwhole-program-vtables -emit-llvm -o - %s | FileCheck --check-prefix=CFI --check-prefix=CFI-VT --check-prefix=ITANIUM --check-prefix=TC-ITANIUM --check-prefix=ITANIUM-MD %s -// RUN: %clang_cc1 -flto -flto-unit -triple x86_64-pc-windows-msvc -fsanitize=cfi-vcall -fsanitize-trap=cfi-vcall -fwhole-program-vtables -emit-llvm -o - %s | FileCheck --check-prefix=CFI --check-prefix=CFI-VT --check-prefix=MS --check-prefix=T
[clang] [Clang] Emit type metadata on vtables when IRPGO instrumentation option is on. (PR #70841)
https://github.com/minglotus-6 edited https://github.com/llvm/llvm-project/pull/70841 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Emit type metadata on vtables when IRPGO instrumentation option is on. (PR #70841)
https://github.com/minglotus-6 updated https://github.com/llvm/llvm-project/pull/70841 >From 99abebc88c09346bd4a70bbf39df13b249116c09 Mon Sep 17 00:00:00 2001 From: Mingming Liu Date: Tue, 31 Oct 2023 01:17:03 -0700 Subject: [PATCH 1/6] [Clang]Emit type metadata when -fprofile-generate is on --- clang/lib/CodeGen/CGVTables.cpp | 2 +- clang/lib/CodeGen/MicrosoftCXXABI.cpp | 3 +- clang/test/CodeGenCXX/type-metadata.cpp | 150 3 files changed, 80 insertions(+), 75 deletions(-) diff --git a/clang/lib/CodeGen/CGVTables.cpp b/clang/lib/CodeGen/CGVTables.cpp index 25e4b1c27932026..895e160dfa8a536 100644 --- a/clang/lib/CodeGen/CGVTables.cpp +++ b/clang/lib/CodeGen/CGVTables.cpp @@ -1312,7 +1312,7 @@ llvm::GlobalObject::VCallVisibility CodeGenModule::GetVCallVisibilityLevel( void CodeGenModule::EmitVTableTypeMetadata(const CXXRecordDecl *RD, llvm::GlobalVariable *VTable, const VTableLayout &VTLayout) { - if (!getCodeGenOpts().LTOUnit) + if (!getCodeGenOpts().LTOUnit && !getCodeGenOpts().hasProfileIRInstr()) return; CharUnits ComponentWidth = GetTargetTypeStoreSize(getVTableComponentType()); diff --git a/clang/lib/CodeGen/MicrosoftCXXABI.cpp b/clang/lib/CodeGen/MicrosoftCXXABI.cpp index d623f8f63ae56c4..740c0b6cb1aac04 100644 --- a/clang/lib/CodeGen/MicrosoftCXXABI.cpp +++ b/clang/lib/CodeGen/MicrosoftCXXABI.cpp @@ -1673,7 +1673,8 @@ void MicrosoftCXXABI::EmitDestructorCall(CodeGenFunction &CGF, void MicrosoftCXXABI::emitVTableTypeMetadata(const VPtrInfo &Info, const CXXRecordDecl *RD, llvm::GlobalVariable *VTable) { - if (!CGM.getCodeGenOpts().LTOUnit) + if (!CGM.getCodeGenOpts().LTOUnit && + !CGM.getCodeGenOpts().hasProfileIRInstr()) return; // TODO: Should VirtualFunctionElimination also be supported here? diff --git a/clang/test/CodeGenCXX/type-metadata.cpp b/clang/test/CodeGenCXX/type-metadata.cpp index 3f8b54af801f8d6..637ec6e6c326ab6 100644 --- a/clang/test/CodeGenCXX/type-metadata.cpp +++ b/clang/test/CodeGenCXX/type-metadata.cpp @@ -1,113 +1,117 @@ // Tests for the cfi-vcall feature: -// RUN: %clang_cc1 -flto -flto-unit -triple x86_64-unknown-linux -fvisibility=hidden -fsanitize=cfi-vcall -fsanitize-trap=cfi-vcall -emit-llvm -o - %s | FileCheck --check-prefix=CFI --check-prefix=CFI-NVT-NO-RV --check-prefix=ITANIUM --check-prefix=ITANIUM-MD --check-prefix=TT-ITANIUM-HIDDEN --check-prefix=NDIAG %s -// RUN: %clang_cc1 -flto -flto-unit -triple x86_64-unknown-linux -fvisibility=hidden -fsanitize=cfi-vcall -emit-llvm -o - %s | FileCheck --check-prefix=CFI --check-prefix=CFI-NVT-NO-RV --check-prefix=ITANIUM --check-prefix=ITANIUM-MD --check-prefix=TT-ITANIUM-HIDDEN --check-prefix=ITANIUM-MD-DIAG --check-prefix=ITANIUM-DIAG --check-prefix=DIAG --check-prefix=DIAG-ABORT %s -// RUN: %clang_cc1 -flto -flto-unit -triple x86_64-unknown-linux -fvisibility=hidden -fsanitize=cfi-vcall -fsanitize-recover=cfi-vcall -emit-llvm -o - %s | FileCheck --check-prefix=CFI --check-prefix=CFI-NVT-NO-RV --check-prefix=ITANIUM --check-prefix=ITANIUM-MD --check-prefix=TT-ITANIUM-HIDDEN --check-prefix=ITANIUM-MD-DIAG --check-prefix=ITANIUM-DIAG --check-prefix=DIAG --check-prefix=DIAG-RECOVER %s -// RUN: %clang_cc1 -flto -flto-unit -triple x86_64-pc-windows-msvc -fsanitize=cfi-vcall -fsanitize-trap=cfi-vcall -emit-llvm -o - %s | FileCheck --check-prefix=CFI --check-prefix=CFI-NVT-NO-RV --check-prefix=MS --check-prefix=TT-MS --check-prefix=NDIAG %s +// RUN: %clang_cc1 -flto -flto-unit -triple x86_64-unknown-linux -fvisibility=hidden -fsanitize=cfi-vcall -fsanitize-trap=cfi-vcall -emit-llvm -o - %s | FileCheck --check-prefix=CFI --check-prefix=CFI-NVT-NO-RV --check-prefix=ITANIUM-TYPEMETADATA --check-prefix=ITANIUM-HIDDEN --check-prefix=ITANIUM-MD --check-prefix=TT-ITANIUM-HIDDEN --check-prefix=NDIAG %s +// RUN: %clang_cc1 -flto -flto-unit -triple x86_64-unknown-linux -fvisibility=hidden -fsanitize=cfi-vcall -emit-llvm -o - %s | FileCheck --check-prefix=CFI --check-prefix=CFI-NVT-NO-RV --check-prefix=ITANIUM-TYPEMETADATA --check-prefix=ITANIUM-HIDDEN --check-prefix=ITANIUM-MD --check-prefix=TT-ITANIUM-HIDDEN --check-prefix=ITANIUM-MD-DIAG --check-prefix=ITANIUM-DIAG --check-prefix=DIAG --check-prefix=DIAG-ABORT %s +// RUN: %clang_cc1 -flto -flto-unit -triple x86_64-unknown-linux -fvisibility=hidden -fsanitize=cfi-vcall -fsanitize-recover=cfi-vcall -emit-llvm -o - %s | FileCheck --check-prefix=CFI --check-prefix=CFI-NVT-NO-RV --check-prefix=ITANIUM-TYPEMETADATA --check-prefix=ITANIUM-HIDDEN --check-prefix=ITANIUM-MD --check-prefix=TT-ITANIUM-HIDDEN --check-prefix=ITANIUM-MD-DIAG --check-prefix=ITANIUM-DIAG --check-prefix=DIAG --check-prefix=DIAG-RECOVER %s +// RUN: %clang_cc1 -flto -flto-unit -triple x86_64-pc-windows-msvc -fsanitize=cfi-vcall -fs
[clang] [Clang] Emit type metadata on vtables when IRPGO instrumentation option is on. (PR #70841)
minglotus-6 wrote: > Somehow I missed those other uses of the ITANIUM prefix. Thanks for cleaning > it up though I think the new names are more descriptive. yep! I hope the new names helps more or less going forward. Thanks for the feedbacks! https://github.com/llvm/llvm-project/pull/70841 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Emit type metadata on vtables when IRPGO instrumentation option is on. (PR #70841)
https://github.com/minglotus-6 closed https://github.com/llvm/llvm-project/pull/70841 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[compiler-rt] [llvm] [clang-tools-extra] [clang] [PGO][GlobalValue][LTO]In GlobalValues::getGlobalIdentifier, use semicolon as delimiter for local-linkage varibles. (PR #74008)
https://github.com/minglotus-6 commented: Resolved comments except the pending discussion about whether to use `.proftext` or`.profraw`. Added `REQUIRES: zlib` for LLVM IR test since the profile reader should be built with zlib support. I'll probably spend sometime to get this test running on my laptop (haven't tried to build llvm on mac before), while waiting for more feedbacks. I'm thinking of submitting it on Thursday or Friday. @ellishg I think the added compiler-rt test (on macosx) should be a test case for issue 74565. https://github.com/llvm/llvm-project/pull/74008 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[llvm] [compiler-rt] [clang] [clang-tools-extra] [PGO][GlobalValue][LTO]In GlobalValues::getGlobalIdentifier, use semicolon as delimiter for local-linkage varibles. (PR #74008)
https://github.com/minglotus-6 edited https://github.com/llvm/llvm-project/pull/74008 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[compiler-rt] [llvm] [clang-tools-extra] [clang] [PGO][GlobalValue][LTO]In GlobalValues::getGlobalIdentifier, use semicolon as delimiter for local-linkage varibles. (PR #74008)
@@ -352,6 +366,8 @@ std::string getIRPGOFuncName(const Function &F, bool InLTO) { return getIRPGOObjectName(F, InLTO, getPGOFuncNameMetadata(F)); } +// DEPRECATED. Use `getIRPGOFuncName`for new code. See that function for minglotus-6 wrote: done. https://github.com/llvm/llvm-project/pull/74008 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[llvm] [clang] [compiler-rt] [clang-tools-extra] [PGO][GlobalValue][LTO]In GlobalValues::getGlobalIdentifier, use semicolon as delimiter for local-linkage varibles. (PR #74008)
@@ -0,0 +1,115 @@ +// This is a regression test for ThinLTO indirect-call-promotion when candidate +// callees need to be imported from another IR module. In the C++ test case, +// `main` calls `global_func` which is defined in another module. `global_func` +// has two indirect callees, one has external linkage and one has local linkage. +// All three functions should be imported into the IR module of main. + +// What the test does: +// - Generate raw profiles from executables and convert it to indexed profiles. +// During the conversion, a profiled callee address in raw profiles will be +// converted to function hash in indexed profiles. +// - Run IRPGO profile use and ThinTLO prelink pipeline and get LLVM bitcodes +// for both cpp files in the C++ test case. +// - Generate ThinLTO summary file with LLVM bitcodes, and run `function-import` pass. +// - Run `pgo-icall-prom` pass for the IR module which needs to import callees. + +// Use lld as linker for more robust test. We need to REQUIRE LLVMgold.so for +// LTO if default linker is GNU ld or gold anyway. +// REQUIRES: lld-available + +// Test should fail where linkage-name and mangled-name diverges, see issue https://github.com/llvm/llvm-project/issues/74565). +// Currently, this name divergence happens on Mach-O object file format, or on +// many (but not all) 32-bit Windows systems. +// +// XFAIL: system-darwin +// +// Mark 32-bit Windows as UNSUPPORTED for now as opposed to XFAIL. This test +// should fail on many (but not all) 32-bit Windows systems and succeed on the +// rest. The flexibility in triple string parsing makes it tricky to capture +// both sets accurately. i[3-9]86 specifies arch as Triple::ArchType::x86, (win32|windows) +// specifies OS as Triple::OS::Win32 +// +// UNSUPPORTED: target={{i[3-9]86-.*-(win32|windows).*}} + +// RUN: rm -rf %t && split-file %s %t && cd %t + +// Do setup work for all below tests. +// Generate raw profiles from real programs and convert it into indexed profiles. +// Use clangxx_pgogen for IR level instrumentation for C++. +// RUN: %clangxx_pgogen -fuse-ld=lld -O2 lib.cpp main.cpp -o main +// RUN: env LLVM_PROFILE_FILE=main.profraw %run ./main +// RUN: llvm-profdata merge main.profraw -o main.profdata + +// Use profile on lib and get bitcode, test that local function callee0 has +// expected !PGOFuncName metadata and external function callee1 doesn't have +// !PGOFuncName metadata. Explicitly skip ICP pass to test ICP happens as +// expected in the IR module that imports functions from lib. +// RUN: %clang -mllvm -disable-icp -fprofile-use=main.profdata -flto=thin -O2 -c lib.cpp -o lib.bc +// RUN: llvm-dis lib.bc -o - | FileCheck %s --check-prefix=PGOName + +// Use profile on main and get bitcode. +// RUN: %clang -fprofile-use=main.profdata -flto=thin -O2 -c main.cpp -o main.bc + +// Run llvm-lto to get summary file. +// RUN: llvm-lto -thinlto -o summary main.bc lib.bc + +// Test the imports of functions. Default import thresholds would work but do +// explicit override to be more futureproof. Note all functions have one basic +// block with a function-entry-count of one, so they are actually hot functions +// per default profile summary hotness cutoff. +// RUN: opt -passes=function-import -import-instr-limit=100 -import-cold-multiplier=1 -summary-file summary.thinlto.bc main.bc -o main.import.bc -print-imports 2>&1 | FileCheck %s --check-prefix=IMPORTS +// Test that '_Z11global_funcv' has indirect calls annotated with value profiles. +// RUN: llvm-dis main.import.bc -o - | FileCheck %s --check-prefix=IR + +// Test that both candidates are ICP'ed and there is no `!VP` in the IR. +// RUN: opt main.import.bc -icp-lto -passes=pgo-icall-prom -S -pass-remarks=pgo-icall-prom 2>&1 | FileCheck %s --check-prefixes=ICP-IR,ICP-REMARK --implicit-check-not="!VP" + +// IMPORTS: main.cpp: Import _Z7callee1v +// IMPORTS: main.cpp: Import _ZL7callee0v.llvm.[[#]] +// IMPORTS: main.cpp: Import _Z11global_funcv + +// PGOName: define dso_local void @_Z7callee1v() {{.*}} !prof ![[#]] { +// PGOName: define internal void @_ZL7callee0v() {{.*}} !prof ![[#]] !PGOFuncName ![[#MD:]] { +// PGOName: ![[#MD]] = !{!"{{.*}}lib.cpp;_ZL7callee0v"} + +// IR-LABEL: define available_externally {{.*}} void @_Z11global_funcv() {{.*}} !prof ![[#]] { +// IR-NEXT: entry: +// IR-NEXT: %0 = load ptr, ptr @calleeAddrs +// IR-NEXT: tail call void %0(), !prof ![[#PROF1:]] +// IR-NEXT: %1 = load ptr, ptr getelementptr inbounds ([2 x ptr], ptr @calleeAddrs, +// IR-NEXT: tail call void %1(), !prof ![[#PROF2:]] + +// The GUID of indirect callee is the MD5 hash of `/path/to/lib.cpp:_ZL7callee0v` minglotus-6 wrote: good catch, done. https://github.com/llvm/llvm-project/pull/74008 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [compiler-rt] [clang] [llvm] [PGO][GlobalValue][LTO]In GlobalValues::getGlobalIdentifier, use semicolon as delimiter for local-linkage varibles. (PR #74008)
minglotus-6 wrote: > Resolved comments except the pending discussion about whether to use > `.proftext` or`.profraw`. > > Added `REQUIRES: zlib` for LLVM IR test since the profile reader should be > built with zlib support. > > I'll probably spend sometime to get this test running on my laptop (haven't > tried to build llvm on mac before), while waiting for more feedbacks. I'm > thinking of submitting it on Thursday or Friday. @ellishg I think the added > compiler-rt test (on macosx) should be a test case for issue 74565. The test failed on mac, but initially on trying to find `dso_local` of line `PGOName: define dso_local void @_Z7callee1v() {{.*}} !prof ![[#]] !PGOFuncName ![[#]] `. - Debugging with `lldb` points to [this comment](https://github.com/llvm/llvm-project/blob/4f1ddf7523c0bbb4075b1682dbe2278080642eee/clang/lib/CodeGen/CodeGenModule.cpp#L1528), saying only COFF and ELF assumes `dso_local`. After making the `dso_local` optional in the [latest commit](https://github.com/llvm/llvm-project/pull/74008/commits/f6a718d80d7a70d60ef8affc282f525e8282), the test started to fail due to the difference of `` and ``. 1. Test failed, when external func `callee1` in expected output doesn't have `!PGOFuncName` metadata but the test output has it. Note `!PGOFuncName` is annotated [only if](https://github.com/llvm/llvm-project/blob/90c23981768713736208d76578ca119a20f6ac60/llvm/lib/ProfileData/InstrProf.cpp#L1262-1265) `PGOName != Func.getName()` 2. Fixing the first error allows me to see the missed import errors. https://github.com/llvm/llvm-project/pull/74008 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [compiler-rt] [llvm] [clang-tools-extra] [PGO][GlobalValue][LTO]In GlobalValues::getGlobalIdentifier, use semicolon as delimiter for local-linkage varibles. (PR #74008)
https://github.com/minglotus-6 edited https://github.com/llvm/llvm-project/pull/74008 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[compiler-rt] [clang] [llvm] [clang-tools-extra] [PGO][GlobalValue][LTO]In GlobalValues::getGlobalIdentifier, use semicolon as delimiter for local-linkage varibles. (PR #74008)
https://github.com/minglotus-6 closed https://github.com/llvm/llvm-project/pull/74008 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits