Skip to content

Conversation

@jhuber6
Copy link
Contributor

@jhuber6 jhuber6 commented Jan 31, 2024

Summary:
A previous patch removed creating these entries in clang in favor of the
backend emitting a callable kernel and having the runtime call that if
present. The support for the old style was kept around in LLVM 18.0 but
now that we have forked to 19.0 we should remove the support.

The effect of this would be that an application linking against a newer
libomptarget that still had the old constructors will no longer be
called. In that case, they can either recompile or use the
libomptarget.so.18 that comes with the previous release.

Summary:
A previous patch removed creating these entries in clang in favor of the
backend emitting a callable kernel and having the runtime call that if
present. The support for the old style was kept around in LLVM 18.0 but
now that we have forked to 19.0 we should remove the support.

The effect of this would be that an application linking against a newer
libomptarget that still had the old constructors will no longer be
called. In that case, they can either recompile or use the
`libomptarget.so.18` that comes with the previous release.
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

Copy link
Contributor

@ronlieb ronlieb left a comment

Choose a reason for hiding this comment

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

lgtm,

@jhuber6 jhuber6 merged commit 2542876 into llvm:main Jan 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

openmp:libomptarget OpenMP offload runtime

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants