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

MaskRay wrote:

Is this used?

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] [clang-tools-extra] [compiler-rt] [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 

https://github.com/MaskRay 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] [llvm] [clang-tools-extra] [compiler-rt] [PGO][GlobalValue][LTO]In GlobalValues::getGlobalIdentifier, use semicolon as delimiter for local-linkage varibles. (PR #74008)

2023-12-08 Thread Fangrui Song via cfe-commits 

https://github.com/MaskRay 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] [llvm] [clang-tools-extra] [compiler-rt] [PGO][GlobalValue][LTO]In GlobalValues::getGlobalIdentifier, use semicolon as delimiter for local-linkage varibles. (PR #74008)

2023-12-08 Thread Fangrui Song 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))

MaskRay wrote:

Nice script!
The clean up commands may be simplified using this scheme

```
#!/bin/bash
OUTDIR=$(dirname $(realpath -s $0))

dir=$(mktemp -d)
trap "rm -r $dir" EXIT
cd $dir
cat > lib.h

[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