Skip to content

[SYCL][ABI-break] Promote extended CG/handler members #6555

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 5 commits into from
Aug 17, 2022
Merged

[SYCL][ABI-break] Promote extended CG/handler members #6555

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
e7604ec
[SYCL][ABI-break] Promote extended CG/handler members
sergey-semenov Aug 8, 2022
ef4cb84
Fix handler layout test
sergey-semenov Aug 10, 2022
ec7cf89
Another fix
sergey-semenov Aug 10, 2022
eb63129
Merge branch 'sycl' into extendedmembers
sergey-semenov Aug 11, 2022
937108a
Merge branch 'sycl' into extendedmembers
sergey-semenov Aug 16, 2022
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: 1 addition & 1 deletion sycl/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ set(SYCL_MINOR_VERSION 7)
set(SYCL_PATCH_VERSION 0)
# Don't forget to re-enable sycl_symbols_windows.dump once we leave ABI-breaking
# window!
set(SYCL_DEV_ABI_VERSION 8)
set(SYCL_DEV_ABI_VERSION 9)
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 I saw this change already in another ABI breaking change today, so someone will get a conflict.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, that goes for all ABI break PRs with the way we handle incrementing SYCL_DEV_ABI_VERSION right now.

if (SYCL_ADD_DEV_VERSION_POSTFIX)
set(SYCL_VERSION_POSTFIX "-${SYCL_DEV_ABI_VERSION}")
endif()
Expand Down
41 changes: 10 additions & 31 deletions sycl/include/sycl/detail/cg.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,17 +88,9 @@ namespace detail {
// with the desired type.
// This allows changing/extending the contents of this vector without changing
// the version.
//

// Used to represent a type of an extended member
enum class ExtendedMembersType : unsigned int {
HANDLER_KERNEL_BUNDLE = 0,
HANDLER_MEM_ADVICE,
// handler_impl is stored in the exended members to avoid breaking ABI.
// TODO: This should be made a member of the handler class once ABI can be
// broken.
HANDLER_IMPL,
};
enum class ExtendedMembersType : unsigned int { PLACEHOLDER_TYPE };

// Holds a pointer to an object of an arbitrary type and an ID value which
// should be used to understand what type pointer points to.
Expand Down Expand Up @@ -244,6 +236,7 @@ class CGExecKernel : public CG {
NDRDescT MNDRDesc;
std::unique_ptr<HostKernelBase> MHostKernel;
std::shared_ptr<detail::kernel_impl> MSyclKernel;
std::shared_ptr<detail::kernel_bundle_impl> MKernelBundle;
std::vector<ArgDesc> MArgs;
std::string MKernelName;
detail::OSModuleHandle MOSModuleHandle;
Expand All @@ -252,6 +245,7 @@ class CGExecKernel : public CG {

CGExecKernel(NDRDescT NDRDesc, std::unique_ptr<HostKernelBase> HKernel,
std::shared_ptr<detail::kernel_impl> SyclKernel,
std::shared_ptr<detail::kernel_bundle_impl> KernelBundle,
std::vector<std::vector<char>> ArgsStorage,
std::vector<detail::AccessorImplPtr> AccStorage,
std::vector<std::shared_ptr<const void>> SharedPtrStorage,
Expand All @@ -266,7 +260,8 @@ class CGExecKernel : public CG {
std::move(SharedPtrStorage), std::move(Requirements),
std::move(Events), std::move(loc)),
MNDRDesc(std::move(NDRDesc)), MHostKernel(std::move(HKernel)),
MSyclKernel(std::move(SyclKernel)), MArgs(std::move(Args)),
MSyclKernel(std::move(SyclKernel)),
MKernelBundle(std::move(KernelBundle)), MArgs(std::move(Args)),
MKernelName(std::move(KernelName)), MOSModuleHandle(OSModuleHandle),
MStreams(std::move(Streams)),
MAuxiliaryResources(std::move(AuxiliaryResources)) {
Expand All @@ -285,15 +280,7 @@ class CGExecKernel : public CG {
}

std::shared_ptr<detail::kernel_bundle_impl> getKernelBundle() {
const std::shared_ptr<std::vector<ExtendedMemberT>> &ExtendedMembers =
getExtendedMembers();
if (!ExtendedMembers)
return nullptr;
for (const ExtendedMemberT &EMember : *ExtendedMembers)
if (ExtendedMembersType::HANDLER_KERNEL_BUNDLE == EMember.MType)
return std::static_pointer_cast<detail::kernel_bundle_impl>(
EMember.MData);
return nullptr;
return MKernelBundle;
}

void clearStreams() { MStreams.clear(); }
Expand Down Expand Up @@ -435,9 +422,10 @@ class CGPrefetchUSM : public CG {
class CGAdviseUSM : public CG {
void *MDst;
size_t MLength;
pi_mem_advice MAdvice;

public:
CGAdviseUSM(void *DstPtr, size_t Length,
CGAdviseUSM(void *DstPtr, size_t Length, pi_mem_advice Advice,
std::vector<std::vector<char>> ArgsStorage,
std::vector<detail::AccessorImplPtr> AccStorage,
std::vector<std::shared_ptr<const void>> SharedPtrStorage,
Expand All @@ -447,19 +435,10 @@ class CGAdviseUSM : public CG {
: CG(Type, std::move(ArgsStorage), std::move(AccStorage),
std::move(SharedPtrStorage), std::move(Requirements),
std::move(Events), std::move(loc)),
MDst(DstPtr), MLength(Length) {}
MDst(DstPtr), MLength(Length), MAdvice(Advice) {}
void *getDst() { return MDst; }
size_t getLength() { return MLength; }

pi_mem_advice getAdvice() {
auto ExtendedMembers = getExtendedMembers();
if (!ExtendedMembers)
return PI_MEM_ADVICE_UNKNOWN;
for (const ExtendedMemberT &EM : *ExtendedMembers)
if ((ExtendedMembersType::HANDLER_MEM_ADVICE == EM.MType) && EM.MData)
return *std::static_pointer_cast<pi_mem_advice>(EM.MData);
return PI_MEM_ADVICE_UNKNOWN;
}
pi_mem_advice getAdvice() { return MAdvice; }
};

class CGInteropTask : public CG {
Expand Down
4 changes: 1 addition & 3 deletions sycl/include/sycl/handler.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1280,9 +1280,6 @@ class __SYCL_EXPORT handler {
kernel_parallel_for_work_group<KernelName, ElementType>(KernelFunc);
}

std::shared_ptr<detail::handler_impl> getHandlerImpl() const;
std::shared_ptr<detail::handler_impl> evictHandlerImpl() const;

void setStateExplicitKernelBundle();
void setStateSpecConstSet();
bool isStateExplicitKernelBundle() const;
Expand Down Expand Up @@ -2565,6 +2562,7 @@ class __SYCL_EXPORT handler {
void mem_advise(const void *Ptr, size_t Length, int Advice);

private:
std::shared_ptr<detail::handler_impl> MImpl;
std::shared_ptr<detail::queue_impl> MQueue;
/// The storage for the arguments passed.
/// We need to store a copy of values that are passed explicitly through
Expand Down
4 changes: 4 additions & 0 deletions sycl/source/detail/handler_impl.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,10 @@ class handler_impl {

// Stores auxiliary resources used by internal operations.
std::vector<std::shared_ptr<const void>> MAuxiliaryResources;

std::shared_ptr<detail::kernel_bundle_impl> MKernelBundle;

pi_mem_advice MAdvice;
};

} // namespace detail
Expand Down
157 changes: 26 additions & 131 deletions sycl/source/handler.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,139 +35,51 @@ handler::handler(std::shared_ptr<detail::queue_impl> Queue,
std::shared_ptr<detail::queue_impl> PrimaryQueue,
std::shared_ptr<detail::queue_impl> SecondaryQueue,
bool IsHost)
: MQueue(std::move(Queue)), MIsHost(IsHost) {
: MImpl(std::make_shared<detail::handler_impl>(std::move(PrimaryQueue),
std::move(SecondaryQueue))),
MQueue(std::move(Queue)), MIsHost(IsHost) {
// Create extended members and insert handler_impl
// TODO: When allowed to break ABI the handler_impl should be made a member
// of the handler class.
auto ExtendedMembers =
std::make_shared<std::vector<detail::ExtendedMemberT>>();
detail::ExtendedMemberT HandlerImplMember = {
detail::ExtendedMembersType::HANDLER_IMPL,
std::make_shared<detail::handler_impl>(std::move(PrimaryQueue),
std::move(SecondaryQueue))};
ExtendedMembers->push_back(std::move(HandlerImplMember));
MSharedPtrStorage.push_back(std::move(ExtendedMembers));
}

static detail::ExtendedMemberT &getHandlerImplMember(
std::vector<std::shared_ptr<const void>> &SharedPtrStorage) {
assert(!SharedPtrStorage.empty());
std::shared_ptr<std::vector<detail::ExtendedMemberT>> ExtendedMembersVec =
detail::convertToExtendedMembers(SharedPtrStorage[0]);
assert(ExtendedMembersVec->size() > 0);
auto &HandlerImplMember = (*ExtendedMembersVec)[0];
assert(detail::ExtendedMembersType::HANDLER_IMPL == HandlerImplMember.MType);
return HandlerImplMember;
}

/// Gets the handler_impl at the start of the extended members.
std::shared_ptr<detail::handler_impl> handler::getHandlerImpl() const {
std::lock_guard<std::mutex> Lock(
detail::GlobalHandler::instance().getHandlerExtendedMembersMutex());
return std::static_pointer_cast<detail::handler_impl>(
getHandlerImplMember(MSharedPtrStorage).MData);
}

/// Gets the handler_impl at the start of the extended members and removes it.
std::shared_ptr<detail::handler_impl> handler::evictHandlerImpl() const {
std::lock_guard<std::mutex> Lock(
detail::GlobalHandler::instance().getHandlerExtendedMembersMutex());
auto &HandlerImplMember = getHandlerImplMember(MSharedPtrStorage);
auto Impl =
std::static_pointer_cast<detail::handler_impl>(HandlerImplMember.MData);

// Reset the data of the member.
// NOTE: We let it stay because removing the front can be expensive. This will
// be improved when the impl is made a member of handler. In fact eviction is
// likely to not be needed when that happens.
HandlerImplMember.MData.reset();

return Impl;
}

// Sets the submission state to indicate that an explicit kernel bundle has been
// set. Throws a sycl::exception with errc::invalid if the current state
// indicates that a specialization constant has been set.
void handler::setStateExplicitKernelBundle() {
getHandlerImpl()->setStateExplicitKernelBundle();
MImpl->setStateExplicitKernelBundle();
}

// Sets the submission state to indicate that a specialization constant has been
// set. Throws a sycl::exception with errc::invalid if the current state
// indicates that an explicit kernel bundle has been set.
void handler::setStateSpecConstSet() {
getHandlerImpl()->setStateSpecConstSet();
}
void handler::setStateSpecConstSet() { MImpl->setStateSpecConstSet(); }

// Returns true if the submission state is EXPLICIT_KERNEL_BUNDLE_STATE and
// false otherwise.
bool handler::isStateExplicitKernelBundle() const {
return getHandlerImpl()->isStateExplicitKernelBundle();
return MImpl->isStateExplicitKernelBundle();
}

// Returns a shared_ptr to kernel_bundle stored in the extended members vector.
// Returns a shared_ptr to the kernel_bundle.
// If there is no kernel_bundle created:
// returns newly created kernel_bundle if Insert is true
// returns shared_ptr(nullptr) if Insert is false
std::shared_ptr<detail::kernel_bundle_impl>
handler::getOrInsertHandlerKernelBundle(bool Insert) const {

std::lock_guard<std::mutex> Lock(
detail::GlobalHandler::instance().getHandlerExtendedMembersMutex());

assert(!MSharedPtrStorage.empty());

std::shared_ptr<std::vector<detail::ExtendedMemberT>> ExtendedMembersVec =
detail::convertToExtendedMembers(MSharedPtrStorage[0]);
// Look for the kernel bundle in extended members
std::shared_ptr<detail::kernel_bundle_impl> KernelBundleImpPtr;
for (const detail::ExtendedMemberT &EMember : *ExtendedMembersVec)
if (detail::ExtendedMembersType::HANDLER_KERNEL_BUNDLE == EMember.MType) {
KernelBundleImpPtr =
std::static_pointer_cast<detail::kernel_bundle_impl>(EMember.MData);
break;
}

// No kernel bundle yet, create one
if (!KernelBundleImpPtr && Insert) {
// Create an empty kernel bundle to add kernels to later
KernelBundleImpPtr =
if (!MImpl->MKernelBundle && Insert) {
MImpl->MKernelBundle =
detail::getSyclObjImpl(get_kernel_bundle<bundle_state::input>(
MQueue->get_context(), {MQueue->get_device()}, {}));

detail::ExtendedMemberT EMember = {
detail::ExtendedMembersType::HANDLER_KERNEL_BUNDLE, KernelBundleImpPtr};
ExtendedMembersVec->push_back(EMember);
}

return KernelBundleImpPtr;
return MImpl->MKernelBundle;
}

// Sets kernel bundle to the provided one. Either replaces existing one or
// create a new entry in the extended members vector.
// Sets kernel bundle to the provided one.
void handler::setHandlerKernelBundle(
const std::shared_ptr<detail::kernel_bundle_impl> &NewKernelBundleImpPtr) {
assert(!MSharedPtrStorage.empty());

std::lock_guard<std::mutex> Lock(
detail::GlobalHandler::instance().getHandlerExtendedMembersMutex());

std::shared_ptr<std::vector<detail::ExtendedMemberT>> ExendedMembersVec =
detail::convertToExtendedMembers(MSharedPtrStorage[0]);

// Look for kernel bundle in extended members and overwrite it.
for (detail::ExtendedMemberT &EMember : *ExendedMembersVec) {
if (detail::ExtendedMembersType::HANDLER_KERNEL_BUNDLE == EMember.MType) {
EMember.MData = NewKernelBundleImpPtr;
return;
}
}

// Kernel bundle was set found so we add it.
detail::ExtendedMemberT EMember = {
detail::ExtendedMembersType::HANDLER_KERNEL_BUNDLE,
NewKernelBundleImpPtr};
ExendedMembersVec->push_back(EMember);
MImpl->MKernelBundle = NewKernelBundleImpPtr;
}

event handler::finalize() {
Expand All @@ -186,7 +98,7 @@ event handler::finalize() {
if (KernelBundleImpPtr) {
// Make sure implicit non-interop kernel bundles have the kernel
if (!KernelBundleImpPtr->isInterop() &&
!getHandlerImpl()->isStateExplicitKernelBundle()) {
!MImpl->isStateExplicitKernelBundle()) {
kernel_id KernelID =
detail::ProgramManager::getInstance().getSYCLKernelID(MKernelName);
bool KernelInserted =
Expand Down Expand Up @@ -299,10 +211,6 @@ event handler::finalize() {
return MLastEvent;
}

// Evict handler_impl from extended members to make sure the command group
// does not keep it alive.
std::shared_ptr<detail::handler_impl> Impl = evictHandlerImpl();

std::unique_ptr<detail::CG> CommandGroup;
switch (type) {
case detail::CG::Kernel:
Expand All @@ -312,11 +220,11 @@ event handler::finalize() {
// assert feature to check if kernel uses assertions
CommandGroup.reset(new detail::CGExecKernel(
std::move(MNDRDesc), std::move(MHostKernel), std::move(MKernel),
std::move(MArgsStorage), std::move(MAccStorage),
std::move(MSharedPtrStorage), std::move(MRequirements),
std::move(MEvents), std::move(MArgs), MKernelName, MOSModuleHandle,
std::move(MStreamStorage), std::move(Impl->MAuxiliaryResources),
MCGType, MCodeLoc));
std::move(MImpl->MKernelBundle), std::move(MArgsStorage),
std::move(MAccStorage), std::move(MSharedPtrStorage),
std::move(MRequirements), std::move(MEvents), std::move(MArgs),
MKernelName, MOSModuleHandle, std::move(MStreamStorage),
std::move(MImpl->MAuxiliaryResources), MCGType, MCodeLoc));
break;
}
case detail::CG::CodeplayInteropTask:
Expand Down Expand Up @@ -365,9 +273,9 @@ event handler::finalize() {
break;
case detail::CG::AdviseUSM:
CommandGroup.reset(new detail::CGAdviseUSM(
MDstPtr, MLength, std::move(MArgsStorage), std::move(MAccStorage),
std::move(MSharedPtrStorage), std::move(MRequirements),
std::move(MEvents), MCGType, MCodeLoc));
MDstPtr, MLength, MImpl->MAdvice, std::move(MArgsStorage),
std::move(MAccStorage), std::move(MSharedPtrStorage),
std::move(MRequirements), std::move(MEvents), MCGType, MCodeLoc));
break;
case detail::CG::CodeplayHostTask:
CommandGroup.reset(new detail::CGHostTask(
Expand Down Expand Up @@ -405,7 +313,7 @@ event handler::finalize() {
}

void handler::addReduction(const std::shared_ptr<const void> &ReduObj) {
getHandlerImpl()->MAuxiliaryResources.push_back(ReduObj);
MImpl->MAuxiliaryResources.push_back(ReduObj);
}

void handler::associateWithHandler(detail::AccessorBaseHost *AccBase,
Expand Down Expand Up @@ -674,7 +582,7 @@ void handler::verifyUsedKernelBundle(const std::string &KernelName) {
return;

// Implicit kernel bundles are populated late so we ignore them
if (!getHandlerImpl()->isStateExplicitKernelBundle())
if (!MImpl->isStateExplicitKernelBundle())
return;

kernel_id KernelID = detail::get_kernel_id_impl(KernelName);
Expand Down Expand Up @@ -741,36 +649,23 @@ void handler::mem_advise(const void *Ptr, size_t Count, int Advice) {
throwIfActionIsCreated();
MDstPtr = const_cast<void *>(Ptr);
MLength = Count;
MImpl->MAdvice = static_cast<pi_mem_advice>(Advice);
setType(detail::CG::AdviseUSM);

assert(!MSharedPtrStorage.empty());

std::lock_guard<std::mutex> Lock(
detail::GlobalHandler::instance().getHandlerExtendedMembersMutex());

std::shared_ptr<std::vector<detail::ExtendedMemberT>> ExtendedMembersVec =
detail::convertToExtendedMembers(MSharedPtrStorage[0]);

detail::ExtendedMemberT EMember = {
detail::ExtendedMembersType::HANDLER_MEM_ADVICE,
std::make_shared<pi_mem_advice>(pi_mem_advice(Advice))};

ExtendedMembersVec->push_back(EMember);
}

void handler::use_kernel_bundle(
const kernel_bundle<bundle_state::executable> &ExecBundle) {

std::shared_ptr<detail::queue_impl> PrimaryQueue =
getHandlerImpl()->MSubmissionPrimaryQueue;
MImpl->MSubmissionPrimaryQueue;
if (PrimaryQueue->get_context() != ExecBundle.get_context())
throw sycl::exception(
make_error_code(errc::invalid),
"Context associated with the primary queue is different from the "
"context associated with the kernel bundle");

std::shared_ptr<detail::queue_impl> SecondaryQueue =
getHandlerImpl()->MSubmissionSecondaryQueue;
MImpl->MSubmissionSecondaryQueue;
if (SecondaryQueue &&
SecondaryQueue->get_context() != ExecBundle.get_context())
throw sycl::exception(
Expand Down
Loading