Skip to content

[Libomptarget] Remove handling of old ctor / dtor entries #80153

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jan 31, 2024
Merged

[Libomptarget] Remove handling of old ctor / dtor entries #80153

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions openmp/libomptarget/include/OffloadEntry.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,6 @@ class OffloadEntryTy {
const char *getNameAsCStr() const { return OffloadEntry.name; }
__tgt_bin_desc *getBinaryDescription() const;

bool isCTor() const { return hasFlags(OMP_DECLARE_TARGET_CTOR); }
bool isDTor() const { return hasFlags(OMP_DECLARE_TARGET_DTOR); }
bool isLink() const { return hasFlags(OMP_DECLARE_TARGET_LINK); }

bool hasFlags(OpenMPOffloadingDeclareTargetFlags Flags) const {
Expand Down
4 changes: 0 additions & 4 deletions openmp/libomptarget/include/PluginManager.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,10 +58,6 @@ struct PluginAdaptorTy {
/// user.
int32_t getNumberOfUserDevices() const { return NumberOfUserDevices; }

/// Add all offload entries described by \p DI to the devices managed by this
/// plugin.
void addOffloadEntries(DeviceImageTy &DI);

/// RTL index, index is the number of devices of other RTLs that were
/// registered before, i.e. the OpenMP index of the first device to be
/// registered with this RTL.
Expand Down
15 changes: 0 additions & 15 deletions openmp/libomptarget/include/device.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,25 +38,13 @@ struct PluginAdaptorTy;
struct __tgt_bin_desc;
struct __tgt_target_table;

///
struct PendingCtorDtorListsTy {
std::list<void *> PendingCtors;
std::list<void *> PendingDtors;
};
typedef std::map<__tgt_bin_desc *, PendingCtorDtorListsTy>
PendingCtorsDtorsPerLibrary;

struct DeviceTy {
int32_t DeviceID;
PluginAdaptorTy *RTL;
int32_t RTLDeviceID;

bool HasMappedGlobalData = false;

PendingCtorsDtorsPerLibrary PendingCtorsDtors;

std::mutex PendingGlobalsMtx;

DeviceTy(PluginAdaptorTy *RTL, int32_t DeviceID, int32_t RTLDeviceID);
// DeviceTy is not copyable
DeviceTy(const DeviceTy &D) = delete;
Expand Down Expand Up @@ -158,9 +146,6 @@ struct DeviceTy {
int32_t destroyEvent(void *Event);
/// }

/// Register \p Entry as an offload entry that is avalable on this device.
void addOffloadEntry(const OffloadEntryTy &Entry);

/// Print all offload entries to stderr.
void dumpOffloadEntries();

Expand Down
4 changes: 0 additions & 4 deletions openmp/libomptarget/include/omptarget.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,10 +91,6 @@ enum tgt_map_type {
enum OpenMPOffloadingDeclareTargetFlags {
/// Mark the entry global as having a 'link' attribute.
OMP_DECLARE_TARGET_LINK = 0x01,
/// Mark the entry kernel as being a global constructor.
OMP_DECLARE_TARGET_CTOR = 0x02,
/// Mark the entry kernel as being a global destructor.
OMP_DECLARE_TARGET_DTOR = 0x04,
/// Mark the entry global as being an indirectly callable function.
OMP_DECLARE_TARGET_INDIRECT = 0x08
};
Expand Down
44 changes: 0 additions & 44 deletions openmp/libomptarget/src/PluginManager.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,19 +89,6 @@ Error PluginAdaptorTy::init() {
return Error::success();
}

void PluginAdaptorTy::addOffloadEntries(DeviceImageTy &DI) {
for (int32_t I = 0, E = getNumberOfUserDevices(); I < E; ++I) {
auto DeviceOrErr = PM->getDevice(DeviceOffset + I);
if (!DeviceOrErr)
FATAL_MESSAGE(DeviceOffset + I, "%s",
toString(DeviceOrErr.takeError()).c_str());

DeviceTy &Device = *DeviceOrErr;
for (__tgt_offload_entry &Entry : DI.entries())
Device.addOffloadEntry(OffloadEntryTy(DI, Entry));
}
}

void PluginManager::init() {
TIMESCOPE();
DP("Loading RTLs...\n");
Expand Down Expand Up @@ -259,9 +246,6 @@ void PluginManager::registerLib(__tgt_bin_desc *Desc) {
PM->TrlTblMtx.unlock();
FoundRTL = &R;

// Register all offload entries with the devices handled by the plugin.
R.addOffloadEntries(DI);

// if an RTL was found we are done - proceed to register the next image
break;
}
Expand Down Expand Up @@ -302,34 +286,6 @@ void PluginManager::unregisterLib(__tgt_bin_desc *Desc) {

FoundRTL = &R;

// Execute dtors for static objects if the device has been used, i.e.
// if its PendingCtors list has been emptied.
for (int32_t I = 0; I < FoundRTL->getNumberOfUserDevices(); ++I) {
auto DeviceOrErr = PM->getDevice(FoundRTL->DeviceOffset + I);
if (!DeviceOrErr)
FATAL_MESSAGE(FoundRTL->DeviceOffset + I, "%s",
toString(DeviceOrErr.takeError()).c_str());

DeviceTy &Device = *DeviceOrErr;
Device.PendingGlobalsMtx.lock();
if (Device.PendingCtorsDtors[Desc].PendingCtors.empty()) {
AsyncInfoTy AsyncInfo(Device);
for (auto &Dtor : Device.PendingCtorsDtors[Desc].PendingDtors) {
int Rc =
target(nullptr, Device, Dtor, CTorDTorKernelArgs, AsyncInfo);
if (Rc != OFFLOAD_SUCCESS) {
DP("Running destructor " DPxMOD " failed.\n", DPxPTR(Dtor));
}
}
// Remove this library's entry from PendingCtorsDtors
Device.PendingCtorsDtors.erase(Desc);
// All constructors have been issued, wait for them now.
if (AsyncInfo.synchronize() != OFFLOAD_SUCCESS)
DP("Failed synchronizing destructors kernels.\n");
}
Device.PendingGlobalsMtx.unlock();
}

DP("Unregistered image " DPxMOD " from RTL " DPxMOD "!\n",
DPxPTR(Img->ImageStart), DPxPTR(R.LibraryHandler.get()));

Expand Down
41 changes: 2 additions & 39 deletions openmp/libomptarget/src/device.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ int HostDataToTargetTy::addEventIfNecessary(DeviceTy &Device,

DeviceTy::DeviceTy(PluginAdaptorTy *RTL, int32_t DeviceID, int32_t RTLDeviceID)
: DeviceID(DeviceID), RTL(RTL), RTLDeviceID(RTLDeviceID),
PendingCtorsDtors(), PendingGlobalsMtx(), MappingInfo(*this) {}
MappingInfo(*this) {}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i think this is extraneous ? duplicated ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's just deleting the previous globals, don't think so.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oops, was downstream merge foolishness, you are correct , its fine


DeviceTy::~DeviceTy() {
if (DeviceID == -1 || !(getInfoLevel() & OMP_INFOTYPE_DUMP_TABLE))
Expand Down Expand Up @@ -297,48 +297,11 @@ int32_t DeviceTy::destroyEvent(void *Event) {
return OFFLOAD_SUCCESS;
}

void DeviceTy::addOffloadEntry(const OffloadEntryTy &Entry) {
std::lock_guard<decltype(PendingGlobalsMtx)> Lock(PendingGlobalsMtx);
DeviceOffloadEntries.getExclusiveAccessor()->insert({Entry.getName(), Entry});
if (Entry.isGlobal())
return;

if (Entry.isCTor()) {
DP("Adding ctor " DPxMOD " to the pending list.\n",
DPxPTR(Entry.getAddress()));
MESSAGE("WARNING: Calling deprecated constructor for entry %s will be "
"removed in a future release \n",
Entry.getNameAsCStr());
PendingCtorsDtors[Entry.getBinaryDescription()].PendingCtors.push_back(
Entry.getAddress());
} else if (Entry.isDTor()) {
// Dtors are pushed in reverse order so they are executed from end
// to beginning when unregistering the library!
DP("Adding dtor " DPxMOD " to the pending list.\n",
DPxPTR(Entry.getAddress()));
MESSAGE("WARNING: Calling deprecated destructor for entry %s will be "
"removed in a future release \n",
Entry.getNameAsCStr());
PendingCtorsDtors[Entry.getBinaryDescription()].PendingDtors.push_front(
Entry.getAddress());
}

if (Entry.isLink()) {
MESSAGE(
"WARNING: The \"link\" attribute is not yet supported for entry: %s!\n",
Entry.getNameAsCStr());
}
}

void DeviceTy::dumpOffloadEntries() {
fprintf(stderr, "Device %i offload entries:\n", DeviceID);
for (auto &It : *DeviceOffloadEntries.getExclusiveAccessor()) {
const char *Kind = "kernel";
if (It.second.isCTor())
Kind = "constructor";
else if (It.second.isDTor())
Kind = "destructor";
else if (It.second.isLink())
if (It.second.isLink())
Kind = "link";
else if (It.second.isGlobal())
Kind = "global var.";
Expand Down
41 changes: 5 additions & 36 deletions openmp/libomptarget/src/omptarget.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -291,36 +291,9 @@ static int initLibrary(DeviceTy &Device) {
}
}

if (Rc != OFFLOAD_SUCCESS) {
if (Rc != OFFLOAD_SUCCESS)
return Rc;
}

/*
* Run ctors for static objects
*/
if (!Device.PendingCtorsDtors.empty()) {
AsyncInfoTy AsyncInfo(Device);
// Call all ctors for all libraries registered so far
for (auto &Lib : Device.PendingCtorsDtors) {
if (!Lib.second.PendingCtors.empty()) {
DP("Has pending ctors... call now\n");
for (auto &Entry : Lib.second.PendingCtors) {
void *Ctor = Entry;
int Rc = target(nullptr, Device, Ctor, CTorDTorKernelArgs, AsyncInfo);
if (Rc != OFFLOAD_SUCCESS) {
REPORT("Running ctor " DPxMOD " failed.\n", DPxPTR(Ctor));
return OFFLOAD_FAIL;
}
}
// Clear the list to indicate that this device has been used
Lib.second.PendingCtors.clear();
DP("Done with pending ctors for lib " DPxMOD "\n", DPxPTR(Lib.first));
}
}
// All constructors have been issued, wait for them now.
if (AsyncInfo.synchronize() != OFFLOAD_SUCCESS)
return OFFLOAD_FAIL;
}
Device.HasMappedGlobalData = true;

static Int32Envar DumpOffloadEntries =
Expand Down Expand Up @@ -435,14 +408,10 @@ bool checkDeviceAndCtors(int64_t &DeviceID, ident_t *Loc) {
FATAL_MESSAGE(DeviceID, "%s", toString(DeviceOrErr.takeError()).data());

// Check whether global data has been mapped for this device
{
std::lock_guard<decltype(DeviceOrErr->PendingGlobalsMtx)> LG(
DeviceOrErr->PendingGlobalsMtx);
if (initLibrary(*DeviceOrErr) != OFFLOAD_SUCCESS) {
REPORT("Failed to init globals on device %" PRId64 "\n", DeviceID);
handleTargetOutcome(false, Loc);
return true;
}
if (initLibrary(*DeviceOrErr) != OFFLOAD_SUCCESS) {
REPORT("Failed to init globals on device %" PRId64 "\n", DeviceID);
handleTargetOutcome(false, Loc);
return true;
}

return false;
Expand Down
9 changes: 1 addition & 8 deletions openmp/libomptarget/test/offloading/ctor_dtor.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,13 +1,6 @@
// RUN: %libomptarget-compilexx-run-and-check-generic
// RUN: %libomptarget-compileoptxx-run-and-check-generic
// RUN: %libomptarget-compilexx-generic && \
// RUN: env OMPTARGET_DUMP_OFFLOAD_ENTRIES=0 %libomptarget-run-generic 2>&1 | \
// RUN: %fcheck-generic --check-prefix=DUMP
//
// DUMP: Device 0 offload entries:
// DUMP-DAG: global var.: s
// DUMP-DAG: kernel: __omp_offloading_{{.*}}_main_
//

#include <cstdio>
struct S {
S() : i(7) {}
Expand Down