Reland "Compute GUIDs once and store in metadata" (#184065) by mtrofin · Pull Request #201849 · llvm/llvm-project

mtrofin · 2026-06-05T14:23:27Z

This reverts #201194, thus relanding @orodley's PR #184065 (and #200323):

This allows us to keep GUIDs consistent across compilation phases which may change the name or linkage type.
See https://discourse.llvm.org/t/rfc-keep-globalvalue-guids-stable/84801

The CFI issues that triggered the original revert are fixed by #201370, together with the addressing of the TODOs in LowerTypeTests.cpp left in the latter. The graphite diff between this change's V1 and V2 shows what's been added:

the TODOs from [CFI][ThinLTO] Remove the need for CFI calculating ThinLTO GUIDs #201370 are done
in LowerTypeTests.cpp, passing !guid when creating a new declaration and when converting a definition to a declaration.
llvm/test/Transforms/LowerTypeTests/export-icall.ll tests also the above def->decl conversion
removed test/Analysis/CtxProfAnalysis/flatten-prethinlink-requires-guid-metadata.ll introduced in [CtxProf] emit fatal usage error when flatten-prethinlink runs without guid metadata #194383 (this was between the revert and this PR), as now the general expectation is that GUID assignment happens appropriately and all passes use getGUID, so there's no reason for CtxProfAnalysis to do something different.

Currently, we reassign GUIDs when CFI promotes internal linkage symbols, which is counter to the goal of the RFC. This is addressed in PR #203171. The reason for this split fix can be explained on compiler-rt/test/cfi/icall/wrong-signature-mixed-lto.c. Here, a module with the exact same source path is compiled twice, under different conditional compilation, to produce 2 objects. Each object defines an internal linkage symbol with the same name (this is install_trap_loop_detection from compiler-rt/test/cfi/trap_loop_signal_handler.inc which is -include-d by both - see how %clang_cfi is defined). The ThinLTO GUID of this symbol will be the same. Its name won't be - because CFI promotes it and renames it using a hash that is based on the IR Module content (rather than the source path). During thinlink, LTO::addThinLTOwill mark each of the 2 exported symbols as prevailing in their corresponding modules. But that is done by associating their GUID to the module. So whichever comes last wins. The other symbol will be marked available externally and its body DCEd later in backend. But each module will refer to its copy of install_trap_loop_detection, and so we end up with a linker error.

As mentioned, the fix is in PR #203171, and this relanding PR just maintains the existing ThinLTO behavior by rewriting the GUIDs. Since we haven't yet leveraged the GUID mechanics for e.g. simplifying PGO, this aspect of this change is essentially NFC.

Co-authored-by: Owen Rodley [email protected]

mtrofin · 2026-06-05T14:23:44Z

This stack of pull requests is managed by Graphite. Learn more about stacking.

github-actions · 2026-06-05T16:19:10Z

🐧 Linux x64 Test Results

208570 tests passed
6790 tests skipped

✅ The build succeeded and all tests passed.

github-actions · 2026-06-09T02:09:40Z

✅ With the latest revision this PR passed the C/C++ code formatter.

github-actions · 2026-06-09T02:51:59Z

🪟 Windows x64 Test Results

139734 tests passed
4885 tests skipped

✅ The build succeeded and all tests passed.

github-actions · 2026-06-16T03:23:59Z

✅ With the latest revision this PR passed the LLVM ABI annotation checker.

mtrofin · 2026-06-23T15:10:36Z

The stage1-ReleaseThinLTO results still don't look great and it seems like the geomean for the clang build is about the same?

Note that some compile time performance effect in thinlto builds should be expected: we serialize/deserialize more. I think the main concern was "is there something else".

re. stage1 - yes, see my earlier response ("for the single-TU compilations, the regression was attributable to modules with lots of global values but not so much IR, where the bitcode serialization of the extra GUID table would start showing up.")

Re. clang-build, there's little change in the overall build timing (I don't think it's a geomean) because the individual compilation outliers like ModuleLinker were few and their negative contribution likely lost, before, in the overall parallel compilation part of the build. The rest is very likely for the same reason (more serialization) seen in the benchmarks builds, as at most we see compilation regressions within the same relative ranges for individual modules.

nikic · 2026-06-23T15:13:31Z

To clarify, the regression I was concerned about is the one on bin/clang-23, i.e. the one during thin linking, not the one on individual files (pre-link).

mtrofin · 2026-06-23T16:18:26Z

Ah, ok. About that one: perf stat -e instructions -r 10 actually shows ~6% improvement with the new change; when I look at cyclesI see a 1% improvement. Wallclock average showed the same. But, the wallclock measurements are actually pretty noisy - as much as 1.31% over the 10 reps.

Looking at the profile itself, (perf record -e instructions):

Before:

+   13.23%    13.23%  ld.lld          lld                   [.] llvm::MD5::body(llvm::ArrayRef<unsigne
+   12.83%    12.82%  ld.lld          lld                   [.] llvm::SimpleBitstreamCursor::Read(unsi
+   10.19%    10.19%  ld.lld          lld                   [.] llvm::SimpleBitstreamCursor::ReadVBR64
+    6.54%     0.00%  ld.lld          [unknown]             [.] 0000000000000000
+    4.64%     4.64%  ld.lld          lld                   [.] llvm::BitstreamCursor::readRecord(unsi
+    2.11%     2.11%  ld.lld          lld                   [.] llvm::xxh3_64bits(unsigned char const*

After:

+   15.02%    15.02%  ld.lld          lld                   [.] llvm::SimpleBitstreamCursor::Read(unsi
+   10.64%    10.64%  ld.lld          lld                   [.] llvm::SimpleBitstreamCursor::ReadVBR64
+    8.56%     0.00%  ld.lld          [unknown]             [.] 0000000000000000
+    6.48%     6.48%  ld.lld          lld                   [.] llvm::BitstreamCursor::readRecord(unsi
+    3.19%     3.19%  ld.lld          lld                   [.] llvm::MD5::body(llvm::ArrayRef<unsigne
+    2.96%     2.96%  ld.lld          lld                   [.] llvm::xxh3_64bits(unsigned char const*

This lines up with the "we serialize more" (and so we deserialize during linking).

Not sure what to make of the perf stat results difference. Perhaps cold runs -> cold caches -> patch is deserialization heavy -> deserialization (of the GUIDs) costs more than MD5 hashing, but then (hot caches) "the tables flip".

teresajohnson

lgtm

llvm-ci · 2026-07-07T23:16:33Z

LLVM Buildbot has detected a new failure on builder libc-riscv32-qemu-yocto-fullbuild-dbg running on rv32gc-qemu-system while building clang,lld,llvm at step 3 "checkout".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/196/builds/20771

Here is the relevant piece of the build log for the reference

Step 3 (checkout) failure: update (failure)
git version 2.45.2
error: RPC failed; HTTP 429 curl 22 The requested URL returned error: 429
fatal: expected flush after ref listing
error: RPC failed; HTTP 429 curl 22 The requested URL returned error: 429
fatal: expected flush after ref listing

llvm-ci · 2026-07-07T23:19:35Z

LLVM Buildbot has detected a new failure on builder amdgpu-hip-blender running on AMD-bb-w-04 while building clang,lld,llvm at step 3 "annotate".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/232/builds/3124

Here is the relevant piece of the build log for the reference

Step 3 (annotate) failure: '../llvm-zorg/zorg/buildbot/builders/annotated/hip-build.sh --jobs=' (failure)
...
+ . ../llvm-zorg/zorg/buildbot/builders/annotated/buildbot-helper.sh
+ set -eu
+ halt_on_failure
+ echo @@@HALT_ON_FAILURE@@@
@@@HALT_ON_FAILURE@@@
+ setup_env
+ build_step 'Setting up the buildbot'
+ echo '@@@BUILD_STEP Setting up the buildbot@@@'
+ BUILDBOT_ROOT=/home/botworker/bbot/hip-blender
@@@BUILD_STEP Setting up the buildbot@@@
++ whoami
+ BUILDBOT_SLAVENAME=botworker
+ AMDGPU_ARCHS='gfx908;gfx90a;gfx1030;gfx1100'
+ EXTERNAL_DIR=/opt/botworker/llvm/External
+ BUILD_DIR=/home/botworker/bbot/hip-blender/botworker/amdgpu-hip-blender
+ DESTDIR=/home/botworker/bbot/hip-blender/botworker/amdgpu-hip-blender/install
+ LLVM_ROOT=/home/botworker/bbot/hip-blender/llvm-project
+ LLVM_REVISION=39dcb0ff91b312bb269334168d5e32be78b60417
+ LLVM_BUILD_DIR=/home/botworker/bbot/hip-blender/botworker/amdgpu-hip-blender/llvm
+ TESTSUITE_ROOT=/home/botworker/bbot/hip-blender/llvm-test-suite
+ TEST_BUILD_DIR=/home/botworker/bbot/hip-blender/botworker/amdgpu-hip-blender/test-suite-build
+ NINJAOPT=
+ echo BUILDBOT_ROOT=/home/botworker/bbot/hip-blender
+ echo BUILDBOT_SLAVENAME=botworker
+ echo 'AMDGPU_ARCHS=gfx908;gfx90a;gfx1030;gfx1100'
+ echo LLVM_ROOT=/home/botworker/bbot/hip-blender/llvm-project
BUILDBOT_ROOT=/home/botworker/bbot/hip-blender
BUILDBOT_SLAVENAME=botworker
AMDGPU_ARCHS=gfx908;gfx90a;gfx1030;gfx1100
LLVM_ROOT=/home/botworker/bbot/hip-blender/llvm-project
TESTSUITE_ROOT=/home/botworker/bbot/hip-blender/llvm-test-suite
EXTERNAL_DIR=/opt/botworker/llvm/External
BUILD_DIR=/home/botworker/bbot/hip-blender/botworker/amdgpu-hip-blender
DESTDIR=/home/botworker/bbot/hip-blender/botworker/amdgpu-hip-blender/install
+ echo TESTSUITE_ROOT=/home/botworker/bbot/hip-blender/llvm-test-suite
+ echo EXTERNAL_DIR=/opt/botworker/llvm/External
+ echo BUILD_DIR=/home/botworker/bbot/hip-blender/botworker/amdgpu-hip-blender
+ echo DESTDIR=/home/botworker/bbot/hip-blender/botworker/amdgpu-hip-blender/install
+ echo LLVM_BUILD_DIR=/home/botworker/bbot/hip-blender/botworker/amdgpu-hip-blender/llvm
+ echo TEST_BUILD_DIR=/home/botworker/bbot/hip-blender/botworker/amdgpu-hip-blender/test-suite-build
LLVM_BUILD_DIR=/home/botworker/bbot/hip-blender/botworker/amdgpu-hip-blender/llvm
TEST_BUILD_DIR=/home/botworker/bbot/hip-blender/botworker/amdgpu-hip-blender/test-suite-build
+ update_llvm
+ '[' '!' -d /home/botworker/bbot/hip-blender/llvm-project ']'
+ build_step 'Updating llvm-project repo'
+ echo '@@@BUILD_STEP Updating llvm-project repo@@@'
@@@BUILD_STEP Updating llvm-project repo@@@
+ git -C /home/botworker/bbot/hip-blender/llvm-project fetch --prune origin
error: RPC failed; HTTP 429 curl 22 The requested URL returned error: 429
fatal: expected flush after ref listing
Step 5 (Updating llvm-project repo) failure: Updating llvm-project repo (failure)
@@@BUILD_STEP Updating llvm-project repo@@@
+ git -C /home/botworker/bbot/hip-blender/llvm-project fetch --prune origin
error: RPC failed; HTTP 429 curl 22 The requested URL returned error: 429
fatal: expected flush after ref listing
program finished with exit code 128
elapsedTime=7.489016

llvm-ci · 2026-07-07T23:53:01Z

LLVM Buildbot has detected a new failure on builder llvm-clang-x86_64-gcc-ubuntu running on sie-linux-worker3 while building clang,lld,llvm at step 6 "test-build-unified-tree-check-all".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/174/builds/37887

Here is the relevant piece of the build log for the reference

Step 6 (test-build-unified-tree-check-all) failure: test (failure)
******************** TEST 'libFuzzer-x86_64-default-Linux :: symbolize-deadlock.test' FAILED ********************
Exit Code: 1

Command Output (stdout):
--
# RUN: at line 1
/home/buildbot/buildbot-root/llvm-clang-x86_64-gcc-ubuntu/build/./bin/clang   -m64  --driver-mode=g++ -O2 -gline-tables-only -fsanitize=address,fuzzer -I/home/buildbot/buildbot-root/llvm-clang-x86_64-gcc-ubuntu/llvm-project/compiler-rt/lib/fuzzer  /home/buildbot/buildbot-root/llvm-clang-x86_64-gcc-ubuntu/llvm-project/compiler-rt/test/fuzzer/SymbolizeDeadlock.cpp -o /home/buildbot/buildbot-root/llvm-clang-x86_64-gcc-ubuntu/build/runtimes/runtimes-bins/compiler-rt/test/fuzzer/X86_64DefaultLinuxConfig/Output/symbolize-deadlock.test.tmp
# executed command: /home/buildbot/buildbot-root/llvm-clang-x86_64-gcc-ubuntu/build/./bin/clang -m64 --driver-mode=g++ -O2 -gline-tables-only -fsanitize=address,fuzzer -I/home/buildbot/buildbot-root/llvm-clang-x86_64-gcc-ubuntu/llvm-project/compiler-rt/lib/fuzzer /home/buildbot/buildbot-root/llvm-clang-x86_64-gcc-ubuntu/llvm-project/compiler-rt/test/fuzzer/SymbolizeDeadlock.cpp -o /home/buildbot/buildbot-root/llvm-clang-x86_64-gcc-ubuntu/build/runtimes/runtimes-bins/compiler-rt/test/fuzzer/X86_64DefaultLinuxConfig/Output/symbolize-deadlock.test.tmp
# note: command had no output on stdout or stderr
# RUN: at line 2
not  /home/buildbot/buildbot-root/llvm-clang-x86_64-gcc-ubuntu/build/runtimes/runtimes-bins/compiler-rt/test/fuzzer/X86_64DefaultLinuxConfig/Output/symbolize-deadlock.test.tmp -rss_limit_mb=20 2>&1
# executed command: not /home/buildbot/buildbot-root/llvm-clang-x86_64-gcc-ubuntu/build/runtimes/runtimes-bins/compiler-rt/test/fuzzer/X86_64DefaultLinuxConfig/Output/symbolize-deadlock.test.tmp -rss_limit_mb=20
# .---command stdout------------
# | INFO: Running with entropic power schedule (0xFF, 100).
# | INFO: Seed: 817533373
# | INFO: Loaded 1 modules   (10 inline 8-bit counters): 10 [0x558156653e48, 0x558156653e52), 
# | INFO: Loaded 1 PC tables (10 PCs): 10 [0x558156653e58,0x558156653ef8), 
# | INFO: -max_len is not provided; libFuzzer will not generate inputs larger than 4096 bytes
# | INFO: A corpus is not provided, starting from an empty corpus
# | #2	INITED cov: 2 ft: 2 corp: 1/1b exec/s: 0 rss: 101Mb
# | #4	NEW    cov: 3 ft: 3 corp: 2/4b lim: 4 exec/s: 0 rss: 101Mb L: 3/3 MS: 2 CrossOver-CrossOver-
# | #572	NEW    cov: 4 ft: 4 corp: 3/12b lim: 8 exec/s: 0 rss: 101Mb L: 8/8 MS: 3 InsertRepeatedBytes-ChangeBinInt-ShuffleBytes-
# | #599	REDUCE cov: 4 ft: 4 corp: 3/9b lim: 8 exec/s: 0 rss: 101Mb L: 5/5 MS: 2 CopyPart-CrossOver-
# | #745	REDUCE cov: 4 ft: 4 corp: 3/7b lim: 8 exec/s: 0 rss: 101Mb L: 3/3 MS: 1 EraseBytes-
# | #37395	REDUCE cov: 5 ft: 5 corp: 4/11b lim: 365 exec/s: 0 rss: 101Mb L: 4/4 MS: 5 EraseBytes-ChangeBit-CopyPart-ShuffleBytes-CopyPart-
# | #37666	REDUCE cov: 5 ft: 5 corp: 4/10b lim: 365 exec/s: 0 rss: 101Mb L: 3/3 MS: 1 EraseBytes-
# | 	NEW_FUNC[1/1]: 0x558156606550 in aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
# | #50692	REDUCE cov: 9 ft: 9 corp: 5/14b lim: 493 exec/s: 0 rss: 101Mb L: 4/4 MS: 1 InsertByte-
# | ==2638296== ERROR: libFuzzer: out-of-memory (used: 101Mb; limit: 20Mb)
# |    To change the out-of-memory limit use -rss_limit_mb=<N>
# | 
# | Live Heap Allocations: 24465951 bytes in 74 chunks; quarantined: 1195178 bytes in 52444 chunks; 17561 other chunks; total chunks: 70079; showing top 95% (at most 8 unique contexts)
# | 21233984 byte(s) (86%) in 1 allocation(s)
# |     #0 0x55815660515d in operator new(unsigned long) /home/buildbot/buildbot-root/llvm-clang-x86_64-gcc-ubuntu/llvm-project/compiler-rt/lib/asan/asan_new_delete.cpp:109:35
# |     #1 0x5581564e821a in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) /home/buildbot/buildbot-root/llvm-clang-x86_64-gcc-ubuntu/llvm-project/compiler-rt/lib/fuzzer/FuzzerDriver.cpp:822:18
# |     #2 0x5581565161c2 in main /home/buildbot/buildbot-root/llvm-clang-x86_64-gcc-ubuntu/llvm-project/compiler-rt/lib/fuzzer/FuzzerMain.cpp:20:10
# |     #3 0x7fa082b0e082 in __libc_start_main /build/glibc-B3wQXB/glibc-2.31/csu/../csu/libc-start.c:308:16
# | 
# | 3148000 byte(s) (12%) in 1 allocation(s)
# |     #0 0x55815660515d in operator new(unsigned long) /home/buildbot/buildbot-root/llvm-clang-x86_64-gcc-ubuntu/llvm-project/compiler-rt/lib/asan/asan_new_delete.cpp:109:35
# |     #1 0x5581564e81f4 in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) /home/buildbot/buildbot-root/llvm-clang-x86_64-gcc-ubuntu/llvm-project/compiler-rt/lib/fuzzer/FuzzerDriver.cpp:821:14
# |     #2 0x5581565161c2 in main /home/buildbot/buildbot-root/llvm-clang-x86_64-gcc-ubuntu/llvm-project/compiler-rt/lib/fuzzer/FuzzerMain.cpp:20:10
# |     #3 0x7fa082b0e082 in __libc_start_main /build/glibc-B3wQXB/glibc-2.31/csu/../csu/libc-start.c:308:16
# | 
# | MS: 2 ShuffleBytes-ChangeByte-
# `-----------------------------
# error: command failed with exit status: 1

--

...

@orodley

This reverts #201194, thus relanding @orodley's PR #184065 (and #200323): > This allows us to keep GUIDs consistent across compilation phases which may change the name or linkage type. > See https://discourse.llvm.org/t/rfc-keep-globalvalue-guids-stable/84801 The CFI issues that triggered the original revert are fixed by #201370, together with the addressing of the TODOs in `LowerTypeTests.cpp` left in the latter. The [graphite diff](https://app.graphite.com/github/pr/llvm/llvm-project/201849/Reland-%23184065) between this change's V1 and V2 shows what's been added: - the `TODO`s from #201370 are done - in LowerTypeTests.cpp, passing `!guid` when creating a new declaration and when converting a definition to a declaration. - `llvm/test/Transforms/LowerTypeTests/export-icall.ll` tests also the above def->decl conversion - removed `test/Analysis/CtxProfAnalysis/flatten-prethinlink-requires-guid-metadata.ll` introduced in #194383 (this was between the revert and this PR), as now the general expectation is that GUID assignment happens appropriately and all passes use `getGUID`, so there's no reason for `CtxProfAnalysis` to do something different. Currently, we reassign GUIDs when CFI promotes internal linkage symbols, which is counter to the goal of the RFC. This is addressed in PR #203171. The reason for this split fix can be explained on `compiler-rt/test/cfi/icall/wrong-signature-mixed-lto.c`. Here, a module with the exact same source path is compiled twice, under different conditional compilation, to produce 2 objects. Each object defines an internal linkage symbol with the same name (this is `install_trap_loop_detection` from `compiler-rt/test/cfi/trap_loop_signal_handler.inc` which is `-include`\-d by both - see how `%clang_cfi` is defined). The ThinLTO GUID of this symbol will be the same. Its name won't be - because CFI promotes it and renames it using a hash that is based on the IR Module content (rather than the source path). During thinlink, `LTO::addThinLTO`will mark each of the 2 exported symbols as prevailing in their corresponding modules. But that is done by associating their GUID to the module. So whichever comes last wins. The other symbol will be marked available externally and its body DCEd later in backend. But each module will refer to its copy of `install_trap_loop_detection`, and so we end up with a linker error. As mentioned, the fix is in PR #203171, and this relanding PR just maintains the existing ThinLTO behavior by rewriting the GUIDs. Since we haven't yet leveraged the GUID mechanics for e.g. simplifying PGO, this aspect of this change is essentially NFC. Co-authored-by: Owen Rodley <[email protected]>

Prior to [#184065](vscode-file://vscode-app/Applications/Visual%20Studio%20Code.app/Contents/Resources/app/out/vs/code/electron-browser/workbench/workbench.html "#184065") (relanded in [#201849](vscode-file://vscode-app/Applications/Visual%20Studio%20Code.app/Contents/Resources/app/out/vs/code/electron-browser/workbench/workbench.html "#201849")), the code introduced in https://reviews.llvm.org/D42922 was using the GUID of an internal linkage GlobalValue to create a module id, which would then be used in a few cuda-specific places (including creating a symbol name suffix). [#184065](vscode-file://vscode-app/Applications/Visual%20Studio%20Code.app/Contents/Resources/app/out/vs/code/electron-browser/workbench/workbench.html "#184065") assumed the linkage of that symbol is external - which it isn't - and, thus, all module IDs computed for this would be identical. The fix is to not rely on GlobalValue GUIDs in the first place. What is needed here is a hash that's specific to this module. So we're creating that and explicitly decoupling that calculation from GlobalValue's notion of GUIDs. The change should be safe wrt uniqueness / hash collisions: the old behavior was computing a hash where the only distinguishing component was the module's `getSourceFileName`. FWIW, there's a stronger module ID we could compute, with `llvm::getUniqueModuleId` (or with a trivial refactoring of it, because it prepends a ".").

Prior to [#184065](vscode-file://vscode-app/Applications/Visual%20Studio%20Code.app/Contents/Resources/app/out/vs/code/electron-browser/workbench/workbench.html "llvm/llvm-project#184065") (relanded in [#201849](vscode-file://vscode-app/Applications/Visual%20Studio%20Code.app/Contents/Resources/app/out/vs/code/electron-browser/workbench/workbench.html "llvm/llvm-project#201849")), the code introduced in https://reviews.llvm.org/D42922 was using the GUID of an internal linkage GlobalValue to create a module id, which would then be used in a few cuda-specific places (including creating a symbol name suffix). [#184065](vscode-file://vscode-app/Applications/Visual%20Studio%20Code.app/Contents/Resources/app/out/vs/code/electron-browser/workbench/workbench.html "llvm/llvm-project#184065") assumed the linkage of that symbol is external - which it isn't - and, thus, all module IDs computed for this would be identical. The fix is to not rely on GlobalValue GUIDs in the first place. What is needed here is a hash that's specific to this module. So we're creating that and explicitly decoupling that calculation from GlobalValue's notion of GUIDs. The change should be safe wrt uniqueness / hash collisions: the old behavior was computing a hash where the only distinguishing component was the module's `getSourceFileName`. FWIW, there's a stronger module ID we could compute, with `llvm::getUniqueModuleId` (or with a trivial refactoring of it, because it prepends a ".").

Prior to [llvm#184065](vscode-file://vscode-app/Applications/Visual%20Studio%20Code.app/Contents/Resources/app/out/vs/code/electron-browser/workbench/workbench.html "llvm#184065") (relanded in [llvm#201849](vscode-file://vscode-app/Applications/Visual%20Studio%20Code.app/Contents/Resources/app/out/vs/code/electron-browser/workbench/workbench.html "llvm#201849")), the code introduced in https://reviews.llvm.org/D42922 was using the GUID of an internal linkage GlobalValue to create a module id, which would then be used in a few cuda-specific places (including creating a symbol name suffix). [llvm#184065](vscode-file://vscode-app/Applications/Visual%20Studio%20Code.app/Contents/Resources/app/out/vs/code/electron-browser/workbench/workbench.html "llvm#184065") assumed the linkage of that symbol is external - which it isn't - and, thus, all module IDs computed for this would be identical. The fix is to not rely on GlobalValue GUIDs in the first place. What is needed here is a hash that's specific to this module. So we're creating that and explicitly decoupling that calculation from GlobalValue's notion of GUIDs. The change should be safe wrt uniqueness / hash collisions: the old behavior was computing a hash where the only distinguishing component was the module's `getSourceFileName`. FWIW, there's a stronger module ID we could compute, with `llvm::getUniqueModuleId` (or with a trivial refactoring of it, because it prepends a ".").

shiltian · 2026-07-21T19:55:55Z

IIUC, this doesn't work with -mlink-builtin-bitcode-postopt if the linked bitcode doesn't have GUIDs assigned to its global variables. Is that expected?

Do we now require -mlink-builtin-bitcode-postopt not to be used with LTO compilation? If so, would it make more sense to emit some kind of warning?

If -mlink-builtin-bitcode-postopt is expected to continue working, should we move the pass to a later point in the pipeline?

@mtrofin

mtrofin · 2026-07-21T21:12:20Z

IIUC, this doesn't work with -mlink-builtin-bitcode-postopt if the linked bitcode doesn't have GUIDs assigned to its global variables. Is that expected?

Do we now require -mlink-builtin-bitcode-postopt not to be used with LTO compilation? If so, would it make more sense to emit some kind of warning?

If -mlink-builtin-bitcode-postopt is expected to continue working, should we move the pass to a later point in the pipeline?

@mtrofin

Do you have a repro? The guids get assigned early so a LTO pass should find them.

shiltian · 2026-07-21T21:57:36Z

Do you have a repro?

I'm not sure how to provide a repro, assuming you don't want to build ROCm components yourself.

The guids get assigned early so a LTO pass should find them.

That's the problem. We probably want to run it later. So for example, I have a bitcode library that is compiled w/o using LTO pipeline. Then this bitcode library will not have GUID. Next, when I compile another file using LTO pipeline, and then link the bitcode library via -mlink-builtin-bitcode-postopt. Because the bitcode library is linked post opt (which is after the GUID assignment pass), but before emitting module summary, then when it emits module summary, some GVs don't have GUIDs.

mtrofin · 2026-07-21T22:35:03Z

Oh, under -mlink-builtin-bitcode-postopt, are you running ThinLTOBitcodeWriterfrom clang/lib/CodeGen/BackendUtil.cpp? Try running AssignGUIDPass before it.

shiltian · 2026-07-22T00:35:13Z

The following can fix the issue.

diff --git a/clang/lib/CodeGen/BackendUtil.cpp b/clang/lib/CodeGen/BackendUtil.cpp
index 2b755fa916e5..c6f6849a37d4 100644
--- a/clang/lib/CodeGen/BackendUtil.cpp
+++ b/clang/lib/CodeGen/BackendUtil.cpp
@@ -92,6 +92,7 @@
 #include "llvm/Transforms/Scalar/GVN.h"
 #include "llvm/Transforms/Scalar/JumpThreading.h"
 #include "llvm/Transforms/Utils/Debugify.h"
+#include "llvm/Transforms/Utils/AssignGUID.h"
 #include "llvm/Transforms/Utils/ModuleUtils.h"
 #include <limits>
 #include <memory>
@@ -1156,6 +1157,7 @@ void EmitAssemblyHelper::RunOptimizationPipeline(
 
   if (Action == Backend_EmitBC || Action == Backend_EmitLL ||
       CodeGenOpts.FatLTO) {
+    MPM.addPass(AssignGUIDPass());
     if (CodeGenOpts.PrepareForThinLTO && !CodeGenOpts.DisableLLVMPasses) {
       if (!TheModule->getModuleFlag("EnableSplitLTOUnit"))
         TheModule->addModuleFlag(llvm::Module::Error, "EnableSplitLTOUnit",

This is not just for ThinLTO though, since full LTO also emits module summary, and I think the assertion I've seen is in the module summary pass. That's why I added it there instead of further down.

mtrofin · 2026-07-22T00:40:30Z

why would you need it for thinlto though - for thinlto, we perform it as part of the regular LLVM PassBuilderPipeline.cpp pipelines.

shiltian · 2026-07-22T00:43:01Z

why would you need it for thinlto though - for thinlto, we perform it as part of the regular LLVM PassBuilderPipeline.cpp pipelines.

I suppose even for ThinLTO, a bitcode library without GUID can also be linked via -mlink-builtin-bitcode-postopt? I didn't try it out for ThinLTO, because we don't support it yet. The change is just based on my understanding.

Are you suggesting something like:

diff --git a/clang/lib/CodeGen/BackendUtil.cpp b/clang/lib/CodeGen/BackendUtil.cpp
index 2b755fa916e5..96c7fe6a76bb 100644
--- a/clang/lib/CodeGen/BackendUtil.cpp
+++ b/clang/lib/CodeGen/BackendUtil.cpp
@@ -92,6 +92,7 @@
 #include "llvm/Transforms/Scalar/GVN.h"
 #include "llvm/Transforms/Scalar/JumpThreading.h"
 #include "llvm/Transforms/Utils/Debugify.h"
+#include "llvm/Transforms/Utils/AssignGUID.h"
 #include "llvm/Transforms/Utils/ModuleUtils.h"
 #include <limits>
 #include <memory>
@@ -1177,6 +1178,7 @@ void EmitAssemblyHelper::RunOptimizationPipeline(
       // targets
       bool EmitLTOSummary = shouldEmitRegularLTOSummary();
       if (EmitLTOSummary) {
+        MPM.addPass(AssignGUIDPass());
         if (!TheModule->getModuleFlag("ThinLTO") && !CodeGenOpts.UnifiedLTO)
           TheModule->addModuleFlag(llvm::Module::Error, "ThinLTO", uint32_t(0));
         if (!TheModule->getModuleFlag("EnableSplitLTOUnit"))

mtrofin · 2026-07-22T00:50:59Z

Normal ThinLTO (in proc or distributed) doesn't need it in clang because it is handled fine in the PassBuilderPipeline.cpp side of things. This feature - -mlink-builtin-bitcode-postop - seems to be constructing a bit of a custom pipeline in Clang, so that's why I think you need a patch specific to that flag.

I'd add it right above LinKInModulesPass(BC)

if (CodeGenOpts.LinkBitcodePostopt) {
    MPM.addPass(AssignGUIDPass());
    MPM.addPass(LinkInModulesPass(BC));
}

shiltian · 2026-07-22T01:49:04Z

#211155

mtrofin force-pushed the users/mtrofin/06-05-reland_184065 branch 2 times, most recently from ad94f68 to a1a789e Compare June 5, 2026 15:21

mtrofin force-pushed the users/mtrofin/06-05-reland_184065 branch 4 times, most recently from 8cd8eb5 to f053482 Compare June 9, 2026 02:08

mtrofin force-pushed the users/mtrofin/06-05-reland_184065 branch from f053482 to 9926393 Compare June 9, 2026 02:10

mtrofin force-pushed the users/mtrofin/06-05-reland_184065 branch 2 times, most recently from 13d3ad0 to f8a1ec9 Compare June 11, 2026 04:58

mtrofin mentioned this pull request Jun 11, 2026

[CFI] Create an external linkage alias instead of promoting internals #203171

Open

mtrofin force-pushed the users/mtrofin/06-05-reland_184065 branch 2 times, most recently from 854a6f9 to 6dc97cf Compare June 16, 2026 03:17

mtrofin force-pushed the users/mtrofin/06-05-reland_184065 branch 2 times, most recently from 6f02138 to c7ff207 Compare June 17, 2026 14:54

mtrofin changed the base branch from main to users/mtrofin/06-17-_coro_handle_aliases June 17, 2026 18:14

mtrofin force-pushed the users/mtrofin/06-05-reland_184065 branch from c7ff207 to 525046e Compare June 17, 2026 18:15

mtrofin mentioned this pull request Jun 17, 2026

[Coro] Handle aliases to coroutines #204408

Merged

mtrofin marked this pull request as ready for review June 17, 2026 18:18

llvmorg-github-actions Bot added lld backend:X86 clang:codegen IR generation bugs: mangling, exceptions, etc. llvm:codegen lld:ELF PGO Profile Guided Optimizations LTO Link time optimization (regular/full LTO or ThinLTO) labels Jun 17, 2026

teresajohnson approved these changes Jun 23, 2026

View reviewed changes

mtrofin force-pushed the users/mtrofin/06-05-reland_184065 branch from 1cbfe8b to 55df97c Compare June 30, 2026 01:29

Reland #184065

e432443

mtrofin force-pushed the users/mtrofin/06-05-reland_184065 branch from 55df97c to e432443 Compare July 6, 2026 14:51

mtrofin merged commit 39dcb0f into main Jul 7, 2026
11 checks passed

mtrofin deleted the users/mtrofin/06-05-reland_184065 branch July 7, 2026 23:04

durin42 mentioned this pull request Jul 8, 2026

Reapply "LLVM 23: Run AssignGUIDPass in some places" rust-lang/rust#158961

Merged

mtrofin mentioned this pull request Jul 13, 2026

[clang][cuda] Use the source filename for module ID #209239

Merged

mtrofin mentioned this pull request Jul 20, 2026

[llvm-reduce] Run AssignGUIDPass when loading BC #208965

Merged

Uh oh!

Conversation

mtrofin commented Jun 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mtrofin commented Jun 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Jun 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🐧 Linux x64 Test Results

Uh oh!

github-actions Bot commented Jun 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Jun 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🪟 Windows x64 Test Results

Uh oh!

github-actions Bot commented Jun 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mtrofin commented Jun 23, 2026

Uh oh!

nikic commented Jun 23, 2026

Uh oh!

mtrofin commented Jun 23, 2026

Uh oh!

teresajohnson left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

llvm-ci commented Jul 7, 2026

Uh oh!

llvm-ci commented Jul 7, 2026

Uh oh!

llvm-ci commented Jul 7, 2026

Uh oh!

shiltian commented Jul 21, 2026

Uh oh!

mtrofin commented Jul 21, 2026

Uh oh!

shiltian commented Jul 21, 2026

Uh oh!

mtrofin commented Jul 21, 2026

Uh oh!

shiltian commented Jul 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mtrofin commented Jul 22, 2026

Uh oh!

shiltian commented Jul 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mtrofin commented Jul 22, 2026

Uh oh!

shiltian commented Jul 22, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

mtrofin commented Jun 5, 2026 •

edited

Loading

mtrofin commented Jun 5, 2026 •

edited

Loading

github-actions Bot commented Jun 5, 2026 •

edited

Loading

github-actions Bot commented Jun 9, 2026 •

edited

Loading

github-actions Bot commented Jun 9, 2026 •

edited

Loading

github-actions Bot commented Jun 16, 2026 •

edited

Loading

shiltian commented Jul 22, 2026 •

edited

Loading

shiltian commented Jul 22, 2026 •

edited

Loading