[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-10 Thread Fangrui Song 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

MaskRay wrote:

`-target ` is deprecated. Use `--target=`. Actually, runtime tests don't 
usually set a `--target=`

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-10 Thread Fangrui Song 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;

MaskRay wrote:

If Name is longer than 64, this would cause two head allocations. It might be 
better to stick with `std::string`.

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 David Li via cfe-commits

david-xl 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.

A follow up patch to fix the tool is fine too (or may be 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


[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