[Openmp-commits] [openmp] [Libomptarget] Pass '-Werror=global-constructors' to the libomptarget build (PR #88531)
Joseph Huber via Openmp-commits
openmp-commits at lists.llvm.org
Fri Apr 12 09:49:25 PDT 2024
https://github.com/jhuber6 updated https://github.com/llvm/llvm-project/pull/88531
>From 82006c0184217d94d4449269c9d18a6263413c87 Mon Sep 17 00:00:00 2001
From: Joseph Huber <huberjn at outlook.com>
Date: Fri, 12 Apr 2024 11:03:18 -0500
Subject: [PATCH] [Libomptarget] Pass '-Werror=global-constructors' to the
libomptarget build
Summary:
A runtime library should not have global constructors. This has caused
many issues in the past so we would make them a hard error if they show
up. This required rewriting the RecordReplay implementation because it
uses a SmallVector internally which can't be made constexpr.
---
openmp/libomptarget/CMakeLists.txt | 5 ++
.../common/src/PluginInterface.cpp | 50 ++++++++++---------
.../kernelreplay/llvm-omp-kernel-replay.cpp | 5 ++
3 files changed, 37 insertions(+), 23 deletions(-)
diff --git a/openmp/libomptarget/CMakeLists.txt b/openmp/libomptarget/CMakeLists.txt
index 531198fae01699..2e223190990aaa 100644
--- a/openmp/libomptarget/CMakeLists.txt
+++ b/openmp/libomptarget/CMakeLists.txt
@@ -36,6 +36,8 @@ include(LibomptargetUtils)
# Get dependencies for the different components of the project.
include(LibomptargetGetDependencies)
+check_cxx_compiler_flag(-Werror=global-constructors OFFLOAD_HAVE_WERROR_CTOR)
+
# LLVM source tree is required at build time for libomptarget
if (NOT LIBOMPTARGET_LLVM_INCLUDE_DIRS)
message(FATAL_ERROR "Missing definition for LIBOMPTARGET_LLVM_INCLUDE_DIRS")
@@ -82,6 +84,9 @@ set(offload_compile_flags -fno-exceptions)
if(NOT LLVM_ENABLE_RTTI)
set(offload_compile_flags ${offload_compile_flags} -fno-rtti)
endif()
+if(OFFLOAD_HAVE_WERROR_CTOR)
+ list(APPEND offload_compile_flags -Werror=global-constructors)
+endif()
# TODO: Consider enabling LTO by default if supported.
# https://cmake.org/cmake/help/latest/module/CheckIPOSupported.html can be used
diff --git a/openmp/libomptarget/plugins-nextgen/common/src/PluginInterface.cpp b/openmp/libomptarget/plugins-nextgen/common/src/PluginInterface.cpp
index b5f3c45c835fdb..30a931aea6ff5d 100644
--- a/openmp/libomptarget/plugins-nextgen/common/src/PluginInterface.cpp
+++ b/openmp/libomptarget/plugins-nextgen/common/src/PluginInterface.cpp
@@ -54,7 +54,7 @@ struct RecordReplayTy {
size_t MemorySize = 0;
size_t TotalSize = 0;
GenericDeviceTy *Device = nullptr;
- std::mutex AllocationLock;
+ std::mutex AllocationLock{};
RRStatusTy Status = RRDeactivated;
bool ReplaySaveOutput = false;
@@ -362,7 +362,10 @@ struct RecordReplayTy {
}
};
-static RecordReplayTy RecordReplay;
+RecordReplayTy &getRecordReplay() {
+ static RecordReplayTy RecordReplay;
+ return RecordReplay;
+}
// Extract the mapping of host function pointers to device function pointers
// from the entry table. Functions marked as 'indirect' in OpenMP will have
@@ -473,7 +476,7 @@ GenericKernelTy::getKernelLaunchEnvironment(
// Ctor/Dtor have no arguments, replaying uses the original kernel launch
// environment. Older versions of the compiler do not generate a kernel
// launch environment.
- if (isCtorOrDtor() || RecordReplay.isReplaying() ||
+ if (isCtorOrDtor() || getRecordReplay().isReplaying() ||
Version < OMP_KERNEL_ARG_MIN_VERSION_WITH_DYN_PTR)
return nullptr;
@@ -562,11 +565,12 @@ Error GenericKernelTy::launch(GenericDeviceTy &GenericDevice, void **ArgPtrs,
// Record the kernel description after we modified the argument count and num
// blocks/threads.
- if (RecordReplay.isRecording()) {
- RecordReplay.saveImage(getName(), getImage());
- RecordReplay.saveKernelInput(getName(), getImage());
- RecordReplay.saveKernelDescr(getName(), Ptrs.data(), KernelArgs.NumArgs,
- NumBlocks, NumThreads, KernelArgs.Tripcount);
+ if (getRecordReplay().isRecording()) {
+ getRecordReplay().saveImage(getName(), getImage());
+ getRecordReplay().saveKernelInput(getName(), getImage());
+ getRecordReplay().saveKernelDescr(getName(), Ptrs.data(),
+ KernelArgs.NumArgs, NumBlocks, NumThreads,
+ KernelArgs.Tripcount);
}
if (auto Err =
@@ -839,8 +843,8 @@ Error GenericDeviceTy::deinit(GenericPluginTy &Plugin) {
delete MemoryManager;
MemoryManager = nullptr;
- if (RecordReplay.isRecordingOrReplaying())
- RecordReplay.deinit();
+ if (getRecordReplay().isRecordingOrReplaying())
+ getRecordReplay().deinit();
if (RPCServer)
if (auto Err = RPCServer->deinitDevice(*this))
@@ -892,7 +896,7 @@ GenericDeviceTy::loadBinary(GenericPluginTy &Plugin,
return std::move(Err);
// Setup the global device memory pool if needed.
- if (!RecordReplay.isReplaying() && shouldSetupDeviceMemoryPool()) {
+ if (!getRecordReplay().isReplaying() && shouldSetupDeviceMemoryPool()) {
uint64_t HeapSize;
auto SizeOrErr = getDeviceHeapSize(HeapSize);
if (SizeOrErr) {
@@ -1307,8 +1311,8 @@ Expected<void *> GenericDeviceTy::dataAlloc(int64_t Size, void *HostPtr,
TargetAllocTy Kind) {
void *Alloc = nullptr;
- if (RecordReplay.isRecordingOrReplaying())
- return RecordReplay.alloc(Size);
+ if (getRecordReplay().isRecordingOrReplaying())
+ return getRecordReplay().alloc(Size);
switch (Kind) {
case TARGET_ALLOC_DEFAULT:
@@ -1344,7 +1348,7 @@ Expected<void *> GenericDeviceTy::dataAlloc(int64_t Size, void *HostPtr,
Error GenericDeviceTy::dataDelete(void *TgtPtr, TargetAllocTy Kind) {
// Free is a noop when recording or replaying.
- if (RecordReplay.isRecordingOrReplaying())
+ if (getRecordReplay().isRecordingOrReplaying())
return Plugin::success();
int Res;
@@ -1397,7 +1401,7 @@ Error GenericDeviceTy::launchKernel(void *EntryPtr, void **ArgPtrs,
KernelArgsTy &KernelArgs,
__tgt_async_info *AsyncInfo) {
AsyncInfoWrapperTy AsyncInfoWrapper(
- *this, RecordReplay.isRecordingOrReplaying() ? nullptr : AsyncInfo);
+ *this, getRecordReplay().isRecordingOrReplaying() ? nullptr : AsyncInfo);
GenericKernelTy &GenericKernel =
*reinterpret_cast<GenericKernelTy *>(EntryPtr);
@@ -1408,9 +1412,9 @@ Error GenericDeviceTy::launchKernel(void *EntryPtr, void **ArgPtrs,
// 'finalize' here to guarantee next record-replay actions are in-sync
AsyncInfoWrapper.finalize(Err);
- if (RecordReplay.isRecordingOrReplaying() &&
- RecordReplay.isSaveOutputEnabled())
- RecordReplay.saveKernelOutputInfo(GenericKernel.getName());
+ if (getRecordReplay().isRecordingOrReplaying() &&
+ getRecordReplay().isSaveOutputEnabled())
+ getRecordReplay().saveKernelOutputInfo(GenericKernel.getName());
return Err;
}
@@ -1630,12 +1634,12 @@ int32_t GenericPluginTy::initialize_record_replay(int32_t DeviceId,
isRecord ? RecordReplayTy::RRStatusTy::RRRecording
: RecordReplayTy::RRStatusTy::RRReplaying;
- if (auto Err = RecordReplay.init(&Device, MemorySize, VAddr, Status,
- SaveOutput, ReqPtrArgOffset)) {
+ if (auto Err = getRecordReplay().init(&Device, MemorySize, VAddr, Status,
+ SaveOutput, ReqPtrArgOffset)) {
REPORT("WARNING RR did not intialize RR-properly with %lu bytes"
"(Error: %s)\n",
MemorySize, toString(std::move(Err)).data());
- RecordReplay.setStatus(RecordReplayTy::RRStatusTy::RRDeactivated);
+ getRecordReplay().setStatus(RecordReplayTy::RRStatusTy::RRDeactivated);
if (!isRecord) {
return OFFLOAD_FAIL;
@@ -1984,8 +1988,8 @@ int32_t GenericPluginTy::get_global(__tgt_device_binary Binary, uint64_t Size,
assert(DevicePtr && "Invalid device global's address");
// Save the loaded globals if we are recording.
- if (RecordReplay.isRecording())
- RecordReplay.addEntry(Name, Size, *DevicePtr);
+ if (getRecordReplay().isRecording())
+ getRecordReplay().addEntry(Name, Size, *DevicePtr);
return OFFLOAD_SUCCESS;
}
diff --git a/openmp/libomptarget/tools/kernelreplay/llvm-omp-kernel-replay.cpp b/openmp/libomptarget/tools/kernelreplay/llvm-omp-kernel-replay.cpp
index 761e04e4c7bbdb..324c123e64057b 100644
--- a/openmp/libomptarget/tools/kernelreplay/llvm-omp-kernel-replay.cpp
+++ b/openmp/libomptarget/tools/kernelreplay/llvm-omp-kernel-replay.cpp
@@ -23,6 +23,9 @@
using namespace llvm;
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wglobal-constructors"
+
cl::OptionCategory ReplayOptions("llvm-omp-kernel-replay Options");
// InputFilename - The filename to read the json description of the kernel.
@@ -52,6 +55,8 @@ static cl::opt<unsigned> NumThreadsOpt("num-threads",
static cl::opt<int32_t> DeviceIdOpt("device-id", cl::desc("Set the device id."),
cl::init(-1), cl::cat(ReplayOptions));
+#pragma GCC diagnostic pop
+
int main(int argc, char **argv) {
cl::HideUnrelatedOptions(ReplayOptions);
cl::ParseCommandLineOptions(argc, argv, "llvm-omp-kernel-replay\n");
More information about the Openmp-commits
mailing list