[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

2023-11-07 Thread Mingming Liu via cfe-commits


@@ -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

2023-11-07 Thread Mingming Liu via cfe-commits


@@ -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)

2023-11-13 Thread Mingming Liu via cfe-commits

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)

2023-11-13 Thread Mingming Liu via cfe-commits


@@ -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)

2023-11-13 Thread Mingming Liu via cfe-commits

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)

2023-11-13 Thread Mingming Liu via cfe-commits


@@ -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)

2023-11-13 Thread Mingming Liu via cfe-commits


@@ -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)

2023-12-04 Thread Mingming Liu via cfe-commits

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)

2023-12-04 Thread Mingming Liu via cfe-commits


@@ -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)

2023-12-04 Thread Mingming Liu via cfe-commits


@@ -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)

2023-12-05 Thread Mingming Liu via cfe-commits


@@ -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)

2023-12-05 Thread Mingming Liu via cfe-commits

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)

2023-12-05 Thread Mingming Liu via cfe-commits

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)

2023-12-05 Thread Mingming Liu via cfe-commits


@@ -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)

2023-12-05 Thread Mingming Liu via cfe-commits

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

2023-12-05 Thread Mingming Liu via cfe-commits

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)

2023-12-06 Thread Mingming Liu via cfe-commits

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)

2023-12-06 Thread Mingming Liu via cfe-commits

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)

2023-12-06 Thread Mingming Liu via cfe-commits

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)

2023-12-06 Thread Mingming Liu via cfe-commits

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)

2023-12-07 Thread Mingming Liu via cfe-commits

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)

2023-12-07 Thread Mingming Liu via cfe-commits

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)

2023-12-07 Thread Mingming Liu via cfe-commits




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)

2023-12-07 Thread Mingming Liu via cfe-commits


@@ -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)

2023-12-07 Thread Mingming Liu via cfe-commits


@@ -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)

2023-12-08 Thread Mingming Liu via cfe-commits

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)

2023-12-08 Thread Mingming Liu via cfe-commits


@@ -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)

2023-12-08 Thread Mingming Liu via cfe-commits


@@ -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)

2023-12-08 Thread Mingming Liu via cfe-commits

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)

2023-12-10 Thread Mingming Liu via cfe-commits

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)

2023-12-10 Thread Mingming Liu via cfe-commits


@@ -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)

2023-12-10 Thread Mingming Liu via cfe-commits


@@ -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)

2023-12-11 Thread Mingming Liu via cfe-commits

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)

2023-12-11 Thread Mingming Liu via cfe-commits

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)

2023-12-11 Thread Mingming Liu via cfe-commits


@@ -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)

2023-12-11 Thread Mingming Liu via cfe-commits


@@ -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)

2023-12-11 Thread Mingming Liu via cfe-commits


@@ -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)

2023-12-11 Thread Mingming Liu via cfe-commits


@@ -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)

2023-12-11 Thread Mingming Liu via cfe-commits


@@ -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)

2023-12-11 Thread Mingming Liu via cfe-commits


@@ -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)

2023-12-11 Thread Mingming Liu via cfe-commits


@@ -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)

2023-12-11 Thread Mingming Liu via cfe-commits


@@ -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)

2023-12-11 Thread Mingming Liu via cfe-commits


@@ -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)

2023-12-11 Thread Mingming Liu via cfe-commits


@@ -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)

2023-12-11 Thread Mingming Liu via cfe-commits


@@ -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)

2023-12-11 Thread Mingming Liu via cfe-commits


@@ -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)

2023-11-14 Thread Mingming Liu via cfe-commits

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

2023-11-15 Thread Mingming Liu via cfe-commits


@@ -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

2023-11-15 Thread Mingming Liu via cfe-commits


@@ -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

2023-11-15 Thread Mingming Liu via cfe-commits


@@ -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

2023-11-15 Thread Mingming Liu via cfe-commits

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

2023-11-15 Thread Mingming Liu via cfe-commits

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

2023-11-15 Thread Mingming Liu via cfe-commits


@@ -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

2023-11-15 Thread Mingming Liu via cfe-commits


@@ -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

2023-11-15 Thread Mingming Liu via cfe-commits


@@ -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

2023-11-15 Thread Mingming Liu via cfe-commits


@@ -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

2023-11-15 Thread Mingming Liu via cfe-commits


@@ -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

2023-11-15 Thread Mingming Liu via cfe-commits


@@ -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

2023-11-15 Thread Mingming Liu via cfe-commits

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

2023-11-16 Thread Mingming Liu via cfe-commits

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

2023-11-17 Thread Mingming Liu via cfe-commits


@@ -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)

2023-10-30 Thread Mingming Liu via cfe-commits


@@ -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)

2023-10-30 Thread Mingming Liu via cfe-commits

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)

2023-10-31 Thread Mingming Liu via cfe-commits

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)

2023-10-31 Thread Mingming Liu via cfe-commits

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)

2023-10-31 Thread Mingming Liu via cfe-commits

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)

2023-10-31 Thread Mingming Liu via cfe-commits

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)

2023-10-31 Thread Mingming Liu via cfe-commits

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)

2023-10-31 Thread Mingming Liu via cfe-commits

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)

2023-10-31 Thread Mingming Liu via cfe-commits

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)

2023-11-01 Thread Mingming Liu via cfe-commits


@@ -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)

2023-11-01 Thread Mingming Liu via cfe-commits


@@ -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)

2023-11-01 Thread Mingming Liu via cfe-commits

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)

2023-11-01 Thread Mingming Liu via cfe-commits

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)

2023-11-01 Thread Mingming Liu via cfe-commits

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)

2023-11-01 Thread Mingming Liu via cfe-commits

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)

2023-11-01 Thread Mingming Liu via cfe-commits

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)

2023-11-01 Thread Mingming Liu via cfe-commits


@@ -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)

2023-11-01 Thread Mingming Liu via cfe-commits


@@ -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)

2023-11-02 Thread Mingming Liu via cfe-commits

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)

2023-11-02 Thread Mingming Liu via cfe-commits

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)

2023-11-02 Thread Mingming Liu via cfe-commits


@@ -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)

2023-11-02 Thread Mingming Liu via cfe-commits

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)

2023-11-02 Thread Mingming Liu via cfe-commits

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)

2023-11-02 Thread Mingming Liu via cfe-commits

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)

2023-11-03 Thread Mingming Liu via cfe-commits


@@ -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)

2023-11-03 Thread Mingming Liu via cfe-commits


@@ -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)

2023-11-03 Thread Mingming Liu via cfe-commits


@@ -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)

2023-11-03 Thread Mingming Liu via cfe-commits


@@ -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)

2023-11-03 Thread Mingming Liu via cfe-commits

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)

2023-11-03 Thread Mingming Liu via cfe-commits

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)

2023-11-03 Thread Mingming Liu via cfe-commits

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)

2023-11-03 Thread Mingming Liu via cfe-commits

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)

2023-12-12 Thread Mingming Liu via cfe-commits

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)

2023-12-12 Thread Mingming Liu via cfe-commits

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)

2023-12-12 Thread Mingming Liu via cfe-commits


@@ -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)

2023-12-12 Thread Mingming Liu via cfe-commits


@@ -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)

2023-12-13 Thread Mingming Liu via cfe-commits

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)

2023-12-13 Thread Mingming Liu via cfe-commits

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)

2023-12-18 Thread Mingming Liu via cfe-commits

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


  1   2   3   4   >