Skip to content

[SYCL] Always let the backend choose the binary #1587

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 7 commits into from
May 12, 2020
Merged

[SYCL] Always let the backend choose the binary #1587

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
bee1066
Always let the backend choose the binary
hiaselhans Apr 25, 2020
b7d6c64
set image type to spir64 for DynRTDeviceBinaryImage
hiaselhans Apr 28, 2020
86eb1e9
fix clang-format
hiaselhans Apr 28, 2020
a18ddcd
set BinaryImage's DeviceTargetSpec based on Format
hiaselhans Apr 28, 2020
4ef3f91
fix clang-format
hiaselhans Apr 28, 2020
3627035
another clang-format fix
hiaselhans Apr 28, 2020
b3ebb95
Merge branch 'sycl' into always_select_binary
hiaselhans May 10, 2020
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
8 changes: 7 additions & 1 deletion sycl/source/detail/device_binary_image.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ DynRTDeviceBinaryImage::DynRTDeviceBinaryImage(
Bin = new pi_device_binary_struct();
Bin->Version = PI_DEVICE_BINARY_VERSION;
Bin->Kind = PI_DEVICE_BINARY_OFFLOAD_KIND_SYCL;
Bin->DeviceTargetSpec = PI_DEVICE_BINARY_TARGET_UNKNOWN;
Bin->CompileOptions = "";
Bin->LinkOptions = "";
Bin->ManifestStart = nullptr;
Expand All @@ -31,6 +30,13 @@ DynRTDeviceBinaryImage::DynRTDeviceBinaryImage(
Bin->EntriesBegin = nullptr;
Bin->EntriesEnd = nullptr;
Bin->Format = pi::getBinaryImageFormat(Bin->BinaryStart, DataSize);
switch (Bin->Format) {
case PI_DEVICE_BINARY_TYPE_SPIRV:
Bin->DeviceTargetSpec = PI_DEVICE_BINARY_TARGET_SPIRV64;
break;
default:
Bin->DeviceTargetSpec = PI_DEVICE_BINARY_TARGET_UNKNOWN;
}
init(Bin);
}

Expand Down
20 changes: 10 additions & 10 deletions sycl/source/detail/program_manager/program_manager.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -637,9 +637,13 @@ ProgramManager::ProgramManager() {
RTDeviceBinaryImage &ProgramManager::getDeviceImage(OSModuleHandle M,
KernelSetId KSId,
const context &Context) {
if (DbgProgMgr > 0)
if (DbgProgMgr > 0) {
std::cerr << ">>> ProgramManager::getDeviceImage(" << M << ", \"" << KSId
<< "\", " << getRawSyclObjImpl(Context) << ")\n";

std::cerr << "available device images:\n";
Copy link
Contributor

Choose a reason for hiding this comment

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

Could help when filtering the output...

Suggested change
std::cerr << "available device images:\n";
std::cerr << "ProgramManager: Available device images:\n";

Copy link
Contributor Author

Choose a reason for hiding this comment

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

actually i just moved this part up because it might be unreachable at it's old place.

If i were to improve those messages i would query for PI_TRACE at runtime instead? what do you think?

Copy link
Contributor

Choose a reason for hiding this comment

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

That would be even better

Copy link
Contributor Author

@hiaselhans hiaselhans May 1, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

@kbobrovs should i replace all DbgProgMgr checks with pi_trace in programmanager?

Copy link
Contributor

Choose a reason for hiding this comment

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

@hiaselhans , sorry for delay. Yes, definitely makes sense. Can be done as a separate PR.

debugPrintBinaryImages();
}
std::lock_guard<std::mutex> Guard(Sync::getGlobalLock());
std::vector<RTDeviceBinaryImageUPtr> &Imgs = *m_DeviceImages[KSId];
const ContextImplPtr Ctx = getSyclObjImpl(Context);
Expand All @@ -652,19 +656,15 @@ RTDeviceBinaryImage &ProgramManager::getDeviceImage(OSModuleHandle M,

// Ask the native runtime under the given context to choose the device image
// it prefers.
if (Imgs.size() > 1) {
std::vector<pi_device_binary> RawImgs(Imgs.size());
for (unsigned I = 0; I < Imgs.size(); I++)
RawImgs[I] = const_cast<pi_device_binary>(&Imgs[I]->getRawData());
std::vector<pi_device_binary> RawImgs(Imgs.size());
for (unsigned I = 0; I < Imgs.size(); I++)
RawImgs[I] = const_cast<pi_device_binary>(&Imgs[I]->getRawData());

Ctx->getPlugin().call<PiApiKind::piextDeviceSelectBinary>(
getFirstDevice(Ctx), RawImgs.data(), (cl_uint)RawImgs.size(), &ImgInd);
}
Ctx->getPlugin().call<PiApiKind::piextDeviceSelectBinary>(
getFirstDevice(Ctx), RawImgs.data(), (cl_uint)RawImgs.size(), &ImgInd);
Img = Imgs[ImgInd].get();

if (DbgProgMgr > 0) {
std::cerr << "available device images:\n";
debugPrintBinaryImages();
std::cerr << "selected device image: " << &Img->getRawData() << "\n";
Img->print();
}
Expand Down