• [Clang][LTO] Assign GUIDs after post-opt bitcode linking #211155 👈
• main

This stack of pull requests is managed by sgh.

@llvm/pr-subscribers-clang-codegen

@llvm/pr-subscribers-clang

Author: Shilei Tian (shiltian)

diff --git a/clang/lib/CodeGen/BackendUtil.cpp b/clang/lib/CodeGen/BackendUtil.cpp
index 2b755fa916e55..068b1b4c262c8 100644
--- a/clang/lib/CodeGen/BackendUtil.cpp
+++ b/clang/lib/CodeGen/BackendUtil.cpp
@@ -91,6 +91,7 @@
 #include "llvm/Transforms/Scalar/EarlyCSE.h"
 #include "llvm/Transforms/Scalar/GVN.h"
 #include "llvm/Transforms/Scalar/JumpThreading.h"
+#include "llvm/Transforms/Utils/AssignGUID.h"
 #include "llvm/Transforms/Utils/Debugify.h"
 #include "llvm/Transforms/Utils/ModuleUtils.h"
 #include <limits>
@@ -1139,8 +1140,10 @@ void EmitAssemblyHelper::RunOptimizationPipeline(
   }
 
   // Link against bitcodes supplied via the -mlink-builtin-bitcode option
-  if (CodeGenOpts.LinkBitcodePostopt)
+  if (CodeGenOpts.LinkBitcodePostopt) {
     MPM.addPass(LinkInModulesPass(BC));
+    MPM.addPass(AssignGUIDPass());
+  }
 
   if (LangOpts.HIPStdPar && !LangOpts.CUDAIsDevice &&
       LangOpts.HIPStdParInterposeAlloc)
diff --git a/clang/test/CodeGen/linking-bitcode-postopt.cpp b/clang/test/CodeGen/linking-bitcode-postopt.cpp
index a0486ed0c9a83..0b0026d25a9c9 100644
--- a/clang/test/CodeGen/linking-bitcode-postopt.cpp
+++ b/clang/test/CodeGen/linking-bitcode-postopt.cpp
@@ -13,7 +13,7 @@
 // RUN:   -mlink-builtin-bitcode-postopt \
 // RUN: %s 2>&1 | FileCheck --check-prefixes=OPTION-POSITIVE %s
 
-// OPTION-POSITIVE: LinkInModulesPass
+// OPTION-POSITIVE: LinkInModulesPass,assign-guid
 
 // RUN: %clang_cc1 -triple amdgcn-- -emit-llvm-bc -o /dev/null \
 // RUN:   -mllvm -print-pipeline-passes \

Seems fine, -mlink-builtin-bitcode is already a hack and this just adds to it.

Right. I have no idea why we want to link into some bitcode libraries after the optimization. There must be a reason. @lamb-j might know more history.

In the past (and probably present), AMDGPUSimplifyLibCalls() optimizes things like sin(x) + cos(x) to sincos(x).

If we pre-linked the device libraries (before AMDGPUSimplifyLibCalls()), we wouldn't pick up the sincos() device library function because it isn't in the original code.

By linking after optimization, we make sure all the needed device library funcs are brought in.

There for a while, we tried linking 2x (once before, once after). But that proved to introduce more overhead than optimization.

I think by default, -mlink-builtin-bitcode does link before optimization. It's only deferred if one also sets -mlink-builtin-bitcode-postopt (which is what Comgr does when linking the device libraries)

In the past (and probably present), AMDGPUSimplifyLibCalls() optimizes things like sin(x) + cos(x) to sincos(x).

If we pre-linked the device libraries (before AMDGPUSimplifyLibCalls()), we wouldn't pick up the sincos() device library function because it isn't in the original code.

By linking after optimization, we make sure all the needed device library funcs are brought in.

There for a while, we tried linking 2x (once before, once after). But that proved to introduce more overhead than optimization.

I think by default, -mlink-builtin-bitcode does link before optimization. It's only deferred if one also sets -mlink-builtin-bitcode-postopt (which is what Comgr does when linking the device libraries)

This is the same problem normal LTO linking has w.r.t. new RTLib calls showing up late. I really wish we had a proper solution for this, but it'd take some complicated handling.