Skip to content

Commit fb0c53d

Browse files
committed
[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.
1 parent fad1470 commit fb0c53d

File tree

4 files changed

+39
-23
lines changed

4 files changed

+39
-23
lines changed

openmp/libomptarget/CMakeLists.txt

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,8 @@ include(LibomptargetUtils)
3636
# Get dependencies for the different components of the project.
3737
include(LibomptargetGetDependencies)
3838

39+
check_cxx_compiler_flag(-Werror=global-constructors OFFLOAD_HAVE_WERROR_CTOR)
40+
3941
# LLVM source tree is required at build time for libomptarget
4042
if (NOT LIBOMPTARGET_LLVM_INCLUDE_DIRS)
4143
message(FATAL_ERROR "Missing definition for LIBOMPTARGET_LLVM_INCLUDE_DIRS")
@@ -82,6 +84,9 @@ set(offload_compile_flags -fno-exceptions)
8284
if(NOT LLVM_ENABLE_RTTI)
8385
set(offload_compile_flags ${offload_compile_flags} -fno-rtti)
8486
endif()
87+
if(OFFLOAD_HAVE_WERROR_CTOR)
88+
list(APPEND offload_compile_flags -Werror=global-constructors)
89+
endif()
8590

8691
# TODO: Consider enabling LTO by default if supported.
8792
# https://cmake.org/cmake/help/latest/module/CheckIPOSupported.html can be used

openmp/libomptarget/plugins-nextgen/common/src/PluginInterface.cpp

Lines changed: 27 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ struct RecordReplayTy {
5454
size_t MemorySize = 0;
5555
size_t TotalSize = 0;
5656
GenericDeviceTy *Device = nullptr;
57-
std::mutex AllocationLock;
57+
std::mutex AllocationLock{};
5858

5959
RRStatusTy Status = RRDeactivated;
6060
bool ReplaySaveOutput = false;
@@ -362,7 +362,10 @@ struct RecordReplayTy {
362362
}
363363
};
364364

365-
static RecordReplayTy RecordReplay;
365+
RecordReplayTy &getRecordReplay() {
366+
static RecordReplayTy RecordReplay;
367+
return RecordReplay;
368+
}
366369

367370
// Extract the mapping of host function pointers to device function pointers
368371
// from the entry table. Functions marked as 'indirect' in OpenMP will have
@@ -473,7 +476,7 @@ GenericKernelTy::getKernelLaunchEnvironment(
473476
// Ctor/Dtor have no arguments, replaying uses the original kernel launch
474477
// environment. Older versions of the compiler do not generate a kernel
475478
// launch environment.
476-
if (isCtorOrDtor() || RecordReplay.isReplaying() ||
479+
if (isCtorOrDtor() || getRecordReplay().isReplaying() ||
477480
Version < OMP_KERNEL_ARG_MIN_VERSION_WITH_DYN_PTR)
478481
return nullptr;
479482

@@ -562,11 +565,12 @@ Error GenericKernelTy::launch(GenericDeviceTy &GenericDevice, void **ArgPtrs,
562565

563566
// Record the kernel description after we modified the argument count and num
564567
// blocks/threads.
565-
if (RecordReplay.isRecording()) {
566-
RecordReplay.saveImage(getName(), getImage());
567-
RecordReplay.saveKernelInput(getName(), getImage());
568-
RecordReplay.saveKernelDescr(getName(), Ptrs.data(), KernelArgs.NumArgs,
569-
NumBlocks, NumThreads, KernelArgs.Tripcount);
568+
if (getRecordReplay().isRecording()) {
569+
getRecordReplay().saveImage(getName(), getImage());
570+
getRecordReplay().saveKernelInput(getName(), getImage());
571+
getRecordReplay().saveKernelDescr(getName(), Ptrs.data(),
572+
KernelArgs.NumArgs, NumBlocks, NumThreads,
573+
KernelArgs.Tripcount);
570574
}
571575

572576
if (auto Err =
@@ -839,8 +843,8 @@ Error GenericDeviceTy::deinit(GenericPluginTy &Plugin) {
839843
delete MemoryManager;
840844
MemoryManager = nullptr;
841845

842-
if (RecordReplay.isRecordingOrReplaying())
843-
RecordReplay.deinit();
846+
if (getRecordReplay().isRecordingOrReplaying())
847+
getRecordReplay().deinit();
844848

845849
if (RPCServer)
846850
if (auto Err = RPCServer->deinitDevice(*this))
@@ -892,7 +896,7 @@ GenericDeviceTy::loadBinary(GenericPluginTy &Plugin,
892896
return std::move(Err);
893897

894898
// Setup the global device memory pool if needed.
895-
if (!RecordReplay.isReplaying() && shouldSetupDeviceMemoryPool()) {
899+
if (!getRecordReplay().isReplaying() && shouldSetupDeviceMemoryPool()) {
896900
uint64_t HeapSize;
897901
auto SizeOrErr = getDeviceHeapSize(HeapSize);
898902
if (SizeOrErr) {
@@ -1307,8 +1311,8 @@ Expected<void *> GenericDeviceTy::dataAlloc(int64_t Size, void *HostPtr,
13071311
TargetAllocTy Kind) {
13081312
void *Alloc = nullptr;
13091313

1310-
if (RecordReplay.isRecordingOrReplaying())
1311-
return RecordReplay.alloc(Size);
1314+
if (getRecordReplay().isRecordingOrReplaying())
1315+
return getRecordReplay().alloc(Size);
13121316

13131317
switch (Kind) {
13141318
case TARGET_ALLOC_DEFAULT:
@@ -1344,7 +1348,7 @@ Expected<void *> GenericDeviceTy::dataAlloc(int64_t Size, void *HostPtr,
13441348

13451349
Error GenericDeviceTy::dataDelete(void *TgtPtr, TargetAllocTy Kind) {
13461350
// Free is a noop when recording or replaying.
1347-
if (RecordReplay.isRecordingOrReplaying())
1351+
if (getRecordReplay().isRecordingOrReplaying())
13481352
return Plugin::success();
13491353

13501354
int Res;
@@ -1397,7 +1401,7 @@ Error GenericDeviceTy::launchKernel(void *EntryPtr, void **ArgPtrs,
13971401
KernelArgsTy &KernelArgs,
13981402
__tgt_async_info *AsyncInfo) {
13991403
AsyncInfoWrapperTy AsyncInfoWrapper(
1400-
*this, RecordReplay.isRecordingOrReplaying() ? nullptr : AsyncInfo);
1404+
*this, getRecordReplay().isRecordingOrReplaying() ? nullptr : AsyncInfo);
14011405

14021406
GenericKernelTy &GenericKernel =
14031407
*reinterpret_cast<GenericKernelTy *>(EntryPtr);
@@ -1408,9 +1412,9 @@ Error GenericDeviceTy::launchKernel(void *EntryPtr, void **ArgPtrs,
14081412
// 'finalize' here to guarantee next record-replay actions are in-sync
14091413
AsyncInfoWrapper.finalize(Err);
14101414

1411-
if (RecordReplay.isRecordingOrReplaying() &&
1412-
RecordReplay.isSaveOutputEnabled())
1413-
RecordReplay.saveKernelOutputInfo(GenericKernel.getName());
1415+
if (getRecordReplay().isRecordingOrReplaying() &&
1416+
getRecordReplay().isSaveOutputEnabled())
1417+
getRecordReplay().saveKernelOutputInfo(GenericKernel.getName());
14141418

14151419
return Err;
14161420
}
@@ -1630,12 +1634,12 @@ int32_t GenericPluginTy::initialize_record_replay(int32_t DeviceId,
16301634
isRecord ? RecordReplayTy::RRStatusTy::RRRecording
16311635
: RecordReplayTy::RRStatusTy::RRReplaying;
16321636

1633-
if (auto Err = RecordReplay.init(&Device, MemorySize, VAddr, Status,
1634-
SaveOutput, ReqPtrArgOffset)) {
1637+
if (auto Err = getRecordReplay().init(&Device, MemorySize, VAddr, Status,
1638+
SaveOutput, ReqPtrArgOffset)) {
16351639
REPORT("WARNING RR did not intialize RR-properly with %lu bytes"
16361640
"(Error: %s)\n",
16371641
MemorySize, toString(std::move(Err)).data());
1638-
RecordReplay.setStatus(RecordReplayTy::RRStatusTy::RRDeactivated);
1642+
getRecordReplay().setStatus(RecordReplayTy::RRStatusTy::RRDeactivated);
16391643

16401644
if (!isRecord) {
16411645
return OFFLOAD_FAIL;
@@ -1984,8 +1988,8 @@ int32_t GenericPluginTy::get_global(__tgt_device_binary Binary, uint64_t Size,
19841988
assert(DevicePtr && "Invalid device global's address");
19851989

19861990
// Save the loaded globals if we are recording.
1987-
if (RecordReplay.isRecording())
1988-
RecordReplay.addEntry(Name, Size, *DevicePtr);
1991+
if (getRecordReplay().isRecording())
1992+
getRecordReplay().addEntry(Name, Size, *DevicePtr);
19891993

19901994
return OFFLOAD_SUCCESS;
19911995
}

openmp/libomptarget/src/device.cpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,8 @@
3333
#include <string>
3434
#include <thread>
3535

36+
[[gnu::constructor(101)]] void ctor() { printf("asdfasdf\n"); }
37+
3638
#ifdef OMPT_SUPPORT
3739
using namespace llvm::omp::target::ompt;
3840
#endif

openmp/libomptarget/tools/kernelreplay/llvm-omp-kernel-replay.cpp

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,9 @@
2323

2424
using namespace llvm;
2525

26+
#pragma GCC diagnostic push
27+
#pragma GCC diagnostic ignored "-Wglobal-constructors"
28+
2629
cl::OptionCategory ReplayOptions("llvm-omp-kernel-replay Options");
2730

2831
// InputFilename - The filename to read the json description of the kernel.
@@ -52,6 +55,8 @@ static cl::opt<unsigned> NumThreadsOpt("num-threads",
5255
static cl::opt<int32_t> DeviceIdOpt("device-id", cl::desc("Set the device id."),
5356
cl::init(-1), cl::cat(ReplayOptions));
5457

58+
#pragma GCC diagnostic pop
59+
5560
int main(int argc, char **argv) {
5661
cl::HideUnrelatedOptions(ReplayOptions);
5762
cl::ParseCommandLineOptions(argc, argv, "llvm-omp-kernel-replay\n");

0 commit comments

Comments
 (0)