Skip to content

[NFCI][SYCL] Change device_image_impl::MDevices to store raw device_impl * #19459

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.

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
Jul 16, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
6 changes: 3 additions & 3 deletions sycl/source/detail/context_impl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -326,13 +326,13 @@ void context_impl::removeAssociatedDeviceGlobal(const void *DeviceGlobalPtr) {
}

void context_impl::addDeviceGlobalInitializer(
ur_program_handle_t Program, const std::vector<device> &Devs,
ur_program_handle_t Program, devices_range Devs,
const RTDeviceBinaryImage *BinImage) {
if (BinImage->getDeviceGlobals().empty())
return;
std::lock_guard<std::mutex> Lock(MDeviceGlobalInitializersMutex);
for (const device &Dev : Devs) {
auto Key = std::make_pair(Program, getSyclObjImpl(Dev)->getHandleRef());
for (device_impl &Dev : Devs) {
auto Key = std::make_pair(Program, Dev.getHandleRef());
auto [Iter, Inserted] = MDeviceGlobalInitializers.emplace(Key, BinImage);
if (Inserted && !Iter->second.MDeviceGlobalsFullyInitialized)
++MDeviceGlobalNotInitializedCnt;
Expand Down
2 changes: 1 addition & 1 deletion sycl/source/detail/context_impl.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,7 @@ class context_impl : public std::enable_shared_from_this<context_impl> {

/// Adds a device global initializer.
void addDeviceGlobalInitializer(ur_program_handle_t Program,
const std::vector<device> &Devs,
devices_range Devs,
const RTDeviceBinaryImage *BinImage);

/// Initializes device globals for a program on the associated queue.
Expand Down
124 changes: 59 additions & 65 deletions sycl/source/detail/device_image_impl.hpp

Large diffs are not rendered by default.

21 changes: 18 additions & 3 deletions sycl/source/detail/device_impl.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -2297,16 +2297,31 @@ struct devices_deref_impl {
using devices_iterator =
variadic_iterator<devices_deref_impl, device,
std::vector<std::shared_ptr<device_impl>>::const_iterator,
std::vector<device>::const_iterator, device_impl *>;
std::vector<device>::const_iterator,
std::vector<device_impl *>::const_iterator,
device_impl *>;

class devices_range : public iterator_range<devices_iterator> {
private:
using Base = iterator_range<devices_iterator>;

public:
using Base::Base;
devices_range(const device &Dev)
: devices_range(&*getSyclObjImpl(Dev), (&*getSyclObjImpl(Dev) + 1), 1) {}
template <typename Container>
decltype(std::declval<Base>().to<Container>()) to() const {
return this->Base::to<Container>();
}

template <typename Container>
std::enable_if_t<std::is_same_v<Container, std::vector<ur_device_handle_t>>,
Container>
to() const {
std::vector<ur_device_handle_t> DeviceHandles;
DeviceHandles.reserve(size());
std::transform(begin(), end(), std::back_inserter(DeviceHandles),
[](device_impl &Dev) { return Dev.getHandleRef(); });
return DeviceHandles;
}
};

#ifndef __INTEL_PREVIEW_BREAKING_CHANGES
Expand Down
9 changes: 9 additions & 0 deletions sycl/source/detail/helpers.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -102,10 +102,19 @@ template <typename iterator> class iterator_range {
iterator_range(IterTy Begin, IterTy End, size_t Size)
: Begin(Begin), End(End), Size(Size) {}

iterator_range()
: iterator_range(static_cast<value_type *>(nullptr),
static_cast<value_type *>(nullptr), 0) {}

template <typename ContainerTy>
iterator_range(const ContainerTy &Container)
: iterator_range(Container.begin(), Container.end(), Container.size()) {}

iterator_range(value_type &Obj) : iterator_range(&Obj, &Obj + 1, 1) {}

iterator_range(const sycl_type &Obj)
: iterator_range(&*getSyclObjImpl(Obj), (&*getSyclObjImpl(Obj) + 1), 1) {}

iterator begin() const { return Begin; }
iterator end() const { return End; }
size_t size() const { return Size; }
Expand Down
23 changes: 23 additions & 0 deletions sycl/source/detail/kernel_bundle_impl.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -1138,6 +1138,29 @@ class kernel_bundle_impl
DeviceGlobalMap MDeviceGlobals{/*OwnerControlledCleanup=*/false};
};

inline bool is_compatible(const std::vector<kernel_id> &KernelIDs,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

source/kernel_bundle.cpp provides bool is_compatible(const std::vector<kernel_id> &KernelIDs, const device &Dev) and is a public SYCL API, but we don't have sycl::device inside program_manager anymore, so what I'm doing here is moving implementation to here and modifying that public API to be just a thin wrapper about this method. That way program_manager can call this without creating sycl::device object.

device_impl &Dev) {
if (KernelIDs.empty())
return true;
// One kernel may be contained in several binary images depending on the
// number of targets. This kernel is compatible with the device if there is
// at least one image (containing this kernel) whose aspects are supported by
// the device and whose target matches the device.
for (const auto &KernelID : KernelIDs) {
std::set<const detail::RTDeviceBinaryImage *> BinImages =
detail::ProgramManager::getInstance().getRawDeviceImages({KernelID});

if (std::none_of(BinImages.begin(), BinImages.end(),
[&](const detail::RTDeviceBinaryImage *Img) {
return doesDevSupportDeviceRequirements(Dev, *Img) &&
doesImageTargetMatchDevice(*Img, Dev);
}))
return false;
}

return true;
}

} // namespace detail
} // namespace _V1
} // namespace sycl
Loading
Loading