llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang-codegen Author: Yury Plyakhin (YuriPlyakhin) <details> <summary>Changes</summary> The SYCL offload wrapper still registers and unregisters the device binary the way the offloading runtimes did before 421085fd740d (#<!-- -->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. --- Full diff: https://github.com/llvm/llvm-project/pull/217173.diff 8 Files Affected: - (modified) clang/lib/CodeGen/CodeGenModule.cpp (+6-9) - (modified) clang/lib/CodeGen/CodeGenModule.h (+3-3) - (modified) clang/lib/CodeGen/CodeGenSYCL.cpp (+6-7) - (modified) clang/test/CodeGenSYCL/offload-include-binary.cpp (+10-8) - (modified) clang/test/OffloadTools/clang-linker-wrapper/linker-wrapper-image.c (+1) - (modified) llvm/include/llvm/Frontend/Offloading/OffloadWrapper.h (+8-8) - (modified) llvm/lib/Frontend/Offloading/OffloadWrapper.cpp (+19-10) - (modified) llvm/test/tools/llvm-offload-wrapper/offload-wrapper.ll (+2-2) ``````````diff diff --git a/clang/lib/CodeGen/CodeGenModule.cpp b/clang/lib/CodeGen/CodeGenModule.cpp index b171418f5d51d..d348e80780ee9 100644 --- a/clang/lib/CodeGen/CodeGenModule.cpp +++ b/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(); diff --git a/clang/lib/CodeGen/CodeGenModule.h b/clang/lib/CodeGen/CodeGenModule.h index f3e0541636638..1f5ecf734c528 100644 --- a/clang/lib/CodeGen/CodeGenModule.h +++ b/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. diff --git a/clang/lib/CodeGen/CodeGenSYCL.cpp b/clang/lib/CodeGen/CodeGenSYCL.cpp index ba7b246df9869..a8c36fb7824be 100644 --- a/clang/lib/CodeGen/CodeGenSYCL.cpp +++ b/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; } diff --git a/clang/test/CodeGenSYCL/offload-include-binary.cpp b/clang/test/CodeGenSYCL/offload-include-binary.cpp index 02c92328d97ae..fada4a303042d 100644 --- a/clang/test/CodeGenSYCL/offload-include-binary.cpp +++ b/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: diff --git a/clang/test/OffloadTools/clang-linker-wrapper/linker-wrapper-image.c b/clang/test/OffloadTools/clang-linker-wrapper/linker-wrapper-image.c index 1f6cc9cc32e8e..238645aace5cf 100644 --- a/clang/test/OffloadTools/clang-linker-wrapper/linker-wrapper-image.c +++ b/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: } diff --git a/llvm/include/llvm/Frontend/Offloading/OffloadWrapper.h b/llvm/include/llvm/Frontend/Offloading/OffloadWrapper.h index bf78372168298..8d0c98f12b24d 100644 --- a/llvm/include/llvm/Frontend/Offloading/OffloadWrapper.h +++ b/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 diff --git a/llvm/lib/Frontend/Offloading/OffloadWrapper.cpp b/llvm/lib/Frontend/Offloading/OffloadWrapper.cpp index 594f0b70b31b2..135d3095ab42a 100644 --- a/llvm/lib/Frontend/Offloading/OffloadWrapper.cpp +++ b/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(); } diff --git a/llvm/test/tools/llvm-offload-wrapper/offload-wrapper.ll b/llvm/test/tools/llvm-offload-wrapper/offload-wrapper.ll index cc053642761e1..420e41adbd115 100644 --- a/llvm/test/tools/llvm-offload-wrapper/offload-wrapper.ll +++ b/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: } `````````` </details> https://github.com/llvm/llvm-project/pull/217173 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
