[clang][SYCL] Align device binary (un)registration with CUDA/HIP/OpenMP (#217173)

The SYCL offload wrapper still registers and unregisters the device
binary the way the offloading runtimes did before 421085f (#86830):
a constructor and a destructor, both at priority 1. That commit moved
OpenMP/CUDA/HIP off a destructor and priority 1, and the reasons apply
to SYCL equally.

With no destructor left to emit, wrapSYCLBinaries() only ever hands back
one function, so shrink its out-parameter to a single Function *. This
also lets CodeGenModule::Release() emit the SYCL constructor next to the
CUDA one instead of ahead of registerGlobalDtorsWithAtExit(), where it
had to sit only because AddGlobalDtor() additions are dropped after that
call.

co-authored by claude

Author: YuriPlyakhin

clang/lib/CodeGen/CodeGenModule.cpp

@@ -1177,15 +1177,6 @@ void CodeGenModule::Release() {
   else
     EmitCXXGlobalInitFunc();
   EmitCXXGlobalCleanUpFunc();
-  if (LangOpts.SYCLIsHost && !CodeGenOpts.OffloadBinaryToEmbedFile.empty()) {
-    auto [SYCLCtorFunction, SYCLDtorFunction] = embedSYCLDeviceBinary();
-    if (SYCLCtorFunction) {
-      // A static initializer may launch a kernel, so the device binaries have
-      // to be registered before any of them run, hence a priority.
-      AddGlobalCtor(SYCLCtorFunction, /*Priority=*/1);
-      AddGlobalDtor(SYCLDtorFunction, /*Priority=*/1);
-    }
-  }
   registerGlobalDtorsWithAtExit();
   EmitCXXThreadLocalInitFunc();
   if (ObjCRuntime)
@@ -1195,6 +1186,12 @@ void CodeGenModule::Release() {
     if (llvm::Function *CudaCtorFunction = CUDARuntime->finalizeModule())
       AddGlobalCtor(CudaCtorFunction);
   }
+  if (LangOpts.SYCLIsHost && !CodeGenOpts.OffloadBinaryToEmbedFile.empty()) {
+    if (llvm::Function *SYCLCtorFunction = embedSYCLDeviceBinary())
+      // A static initializer may launch a kernel, so the device binary has to
+      // be registered before any of them run, hence a priority.
+      AddGlobalCtor(SYCLCtorFunction, /*Priority=*/101);
+  }
   if (OpenMPRuntime) {
     OpenMPRuntime->createOffloadEntriesAndInfoMetadata();
     OpenMPRuntime->clear();

clang/lib/CodeGen/CodeGenModule.h

@@ -2181,9 +2181,9 @@ class CodeGenModule : public CodeGenTypeCache {
 
   /// Embed the finalized SYCL device binary named by -foffload-include-binary
   /// into the host module.
-  /// \return the functions that register and unregister the binary with the
-  /// runtime, both null if the binary could not be read.
-  std::pair<llvm::Function *, llvm::Function *> embedSYCLDeviceBinary();
+  /// \return the function that registers the binary with the runtime, or null
+  /// if the binary could not be read.
+  llvm::Function *embedSYCLDeviceBinary();
 
   /// Determine whether the definition must be emitted; if this returns \c
   /// false, the definition can be emitted lazily if it's used.

clang/lib/CodeGen/CodeGenSYCL.cpp

@@ -89,24 +89,23 @@ void CodeGenModule::EmitSYCLKernelCaller(const FunctionDecl *KernelEntryPointFn,
   CGF.FinishFunction();
 }
 
-std::pair<llvm::Function *, llvm::Function *>
-CodeGenModule::embedSYCLDeviceBinary() {
+llvm::Function *CodeGenModule::embedSYCLDeviceBinary() {
   StringRef FileName = getCodeGenOpts().OffloadBinaryToEmbedFile;
   auto BufferOrErr = getFileSystem()->getBufferForFile(FileName);
   if (std::error_code EC = BufferOrErr.getError()) {
     getDiags().Report(diag::err_cannot_open_file) << FileName << EC.message();
-    return {nullptr, nullptr};
+    return nullptr;
   }
   std::unique_ptr<llvm::MemoryBuffer> Buffer = std::move(BufferOrErr.get());
-  std::pair<llvm::Function *, llvm::Function *> RegistrationFuncs;
+  llvm::Function *RegistrationFunc = nullptr;
   if (llvm::Error Err = llvm::offloading::wrapSYCLBinaries(
           getModule(),
           ArrayRef<char>(Buffer->getBufferStart(), Buffer->getBufferSize()),
           llvm::offloading::SYCLJITOptions(), /*IsFinalizedImage=*/true,
-          &RegistrationFuncs)) {
+          &RegistrationFunc)) {
     getDiags().Report(diag::err_fe_error_backend)
         << llvm::toString(std::move(Err));
-    return {nullptr, nullptr};
+    return nullptr;
   }
-  return RegistrationFuncs;
+  return RegistrationFunc;
 }

clang/test/CodeGenSYCL/offload-include-binary.cpp

@@ -1,18 +1,20 @@
 // REQUIRES: x86-registered-target
 
 // Verify that -foffload-include-binary embeds the finalized SYCL device
-// binary into the host module and emits the registration/unregistration
-// constructors and destructors expected by the SYCL runtime.
+// binary into the host module and emits the constructor that registers it with
+// the SYCL runtime. Unregistration is done from 'atexit', so no global
+// destructor is emitted for it.
 // The binary is already finalized, so it must not land in ".llvm.offloading".
 // RUN: echo -n 'FAKE_SYCL_DEVICE_IMAGE' > %t.bin
 // RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fsycl-is-host \
 // RUN:   -foffload-include-binary %t.bin -emit-llvm %s -o - \
 // RUN:   | FileCheck %s --implicit-check-not='.llvm.offloading' \
-// RUN:     --implicit-check-not='llvm.global_ctors.'
+// RUN:     --implicit-check-not='llvm.global_ctors.' \
+// RUN:     --implicit-check-not='llvm.global_dtors'
 
-// The registration functions have to merge into the constructor and destructor
-// lists the rest of the translation unit contributes to, so object emission
-// must succeed for a translation unit that has its own static initializers.
+// The registration function has to merge into the constructor list the rest of
+// the translation unit contributes to, so object emission must succeed for a
+// translation unit that has its own static initializers.
 // RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fsycl-is-host \
 // RUN:   -foffload-include-binary %t.bin -emit-obj %s -o %t.o
 
@@ -39,11 +41,11 @@ void f() {}
 // CHECK: @.sycl_offloading.binary = internal unnamed_addr constant [22 x i8] c"FAKE_SYCL_DEVICE_IMAGE", section ".sycl_fatbin"
 // CHECK:      @llvm.global_ctors = appending global [2 x { i32, ptr, ptr }]
 // CHECK-SAME:   i32 65535, ptr @_GLOBAL__sub_I_
-// CHECK-SAME:   i32 1, ptr @sycl.descriptor_reg
-// CHECK:      @llvm.global_dtors = {{.*}}@sycl.descriptor_unreg
+// CHECK-SAME:   i32 101, ptr @sycl.descriptor_reg
 // CHECK:      define internal void @sycl.descriptor_reg()
 // CHECK-NEXT: entry:
 // CHECK-NEXT:   call void @__sycl_register_lib(ptr @.sycl_offloading.binary, i64 22)
+// CHECK-NEXT:   call i32 @atexit(ptr @sycl.descriptor_unreg)
 // CHECK-NEXT:   ret void
 // CHECK:      define internal void @sycl.descriptor_unreg()
 // CHECK-NEXT: entry:

clang/test/OffloadTools/clang-linker-wrapper/linker-wrapper-image.c

@@ -316,6 +316,7 @@
 //      SYCL: define internal void @sycl.descriptor_reg() section ".text.startup" {
 // SYCL-NEXT: entry:
 // SYCL-NEXT:   call void @__sycl_register_lib(ptr @.sycl_offloading.binary, i64 0)
+// SYCL-NEXT:   %0 = call i32 @atexit(ptr @sycl.descriptor_unreg)
 // SYCL-NEXT:   ret void
 // SYCL-NEXT: }
 

llvm/include/llvm/Frontend/Offloading/OffloadWrapper.h

@@ -70,14 +70,14 @@ struct SYCLJITOptions {
 ///  use by a runtime for JIT compilation. Not used for AOT.
 /// \param IsFinalizedImage True when \p Buffer holds an already finalized
 ///  device image, which must not be device-linked again.
-/// \param RegistrationFuncs When given, receives the functions that register
-///  and unregister the binary with the runtime instead of them being appended
-///  to llvm.global_ctors and llvm.global_dtors. A caller has to add them
-///  to those lists itself.
-LLVM_ABI llvm::Error wrapSYCLBinaries(
-    llvm::Module &M, llvm::ArrayRef<char> Buffer,
-    SYCLJITOptions Options = SYCLJITOptions(), bool IsFinalizedImage = false,
-    std::pair<llvm::Function *, llvm::Function *> *RegistrationFuncs = nullptr);
+/// \param RegistrationFunc When given, receives the function that registers the
+///  binary with the runtime instead of it being appended to llvm.global_ctors.
+///  A caller has to add it to that list itself.
+LLVM_ABI llvm::Error
+wrapSYCLBinaries(llvm::Module &M, llvm::ArrayRef<char> Buffer,
+                 SYCLJITOptions Options = SYCLJITOptions(),
+                 bool IsFinalizedImage = false,
+                 llvm::Function **RegistrationFunc = nullptr);
 
 } // namespace offloading
 } // namespace llvm

llvm/lib/Frontend/Offloading/OffloadWrapper.cpp

@@ -676,13 +676,25 @@ class SYCLWrapper {
     FunctionCallee RegFuncC =
         M.getOrInsertFunction("__sycl_register_lib", RegFuncTy);
 
+    FunctionType *AtExitTy =
+        FunctionType::get(Type::getInt32Ty(C), PtrTy, /*isVarArg=*/false);
+    FunctionCallee AtExit = M.getOrInsertFunction("atexit", AtExitTy);
+
+    Function *UnregFunc = createUnregisterFunction(Start, Size);
+
     IRBuilder<> Builder(BasicBlock::Create(C, "entry", Func));
     Builder.CreateCall(RegFuncC, {Start, Size});
+
+    // Unregister with 'atexit'. The handler is installed after
+    // __sycl_register_lib has brought the runtime's own exit-time cleanup into
+    // the atexit chain, so it is ordered ahead of that cleanup.
+    Builder.CreateCall(AtExit, UnregFunc);
     Builder.CreateRetVoid();
 
     return Func;
   }
 
+private:
   Function *createUnregisterFunction(Constant *Start, Constant *Size) {
     FunctionType *FuncTy =
         FunctionType::get(Type::getVoidTy(C), /*isVarArg*/ false);
@@ -705,7 +717,6 @@ class SYCLWrapper {
     return Func;
   }
 
-private:
   Module &M;
   LLVMContext &C;
   SYCLJITOptions Options;
@@ -753,20 +764,18 @@ Error offloading::wrapHIPBinary(Module &M, ArrayRef<char> Image,
   return Error::success();
 }
 
-Error llvm::offloading::wrapSYCLBinaries(
-    llvm::Module &M, ArrayRef<char> Buffer, SYCLJITOptions Options,
-    bool IsFinalizedImage,
-    std::pair<Function *, Function *> *RegistrationFuncs) {
+Error llvm::offloading::wrapSYCLBinaries(llvm::Module &M, ArrayRef<char> Buffer,
+                                         SYCLJITOptions Options,
+                                         bool IsFinalizedImage,
+                                         Function **RegistrationFunc) {
   SYCLWrapper W(M, Options, IsFinalizedImage);
   auto [Start, Size] = W.embedBinary(Buffer);
   Function *RegisterFunc = W.createRegisterFatbinFunction(Start, Size);
-  Function *UnregisterFunc = W.createUnregisterFunction(Start, Size);
-  if (RegistrationFuncs) {
-    *RegistrationFuncs = {RegisterFunc, UnregisterFunc};
+  if (RegistrationFunc) {
+    *RegistrationFunc = RegisterFunc;
     return Error::success();
   }
 
-  appendToGlobalCtors(M, RegisterFunc, /*Priority*/ 1);
-  appendToGlobalDtors(M, UnregisterFunc, /*Priority*/ 1);
+  appendToGlobalCtors(M, RegisterFunc, /*Priority=*/101);
   return Error::success();
 }

llvm/test/tools/llvm-offload-wrapper/offload-wrapper.ll

@@ -125,12 +125,12 @@
 ; RUN: llvm-dis %t.bc -o - | FileCheck %s --check-prefix=SYCL
 
 ;      SYCL: @.sycl_offloading.binary = internal unnamed_addr constant [[[SIZE:[0-9]+]] x i8] c"{{.*}}", section ".llvm.offloading"
-; SYCL-NEXT: @llvm.global_ctors = appending global [1 x { i32, ptr, ptr }] [{ i32, ptr, ptr } { i32 1, ptr @sycl.descriptor_reg, ptr null }]
-; SYCL-NEXT: @llvm.global_dtors = appending global [1 x { i32, ptr, ptr }] [{ i32, ptr, ptr } { i32 1, ptr @sycl.descriptor_unreg, ptr null }]
+; SYCL-NEXT: @llvm.global_ctors = appending global [1 x { i32, ptr, ptr }] [{ i32, ptr, ptr } { i32 101, ptr @sycl.descriptor_reg, ptr null }]
 
 ;      SYCL: define internal void @sycl.descriptor_reg() section ".text.startup" {
 ; SYCL-NEXT: entry:
 ; SYCL-NEXT:   call void @__sycl_register_lib(ptr @.sycl_offloading.binary, i64 [[SIZE]])
+; SYCL-NEXT:   %0 = call i32 @atexit(ptr @sycl.descriptor_unreg)
 ; SYCL-NEXT:   ret void
 ; SYCL-NEXT: }