Skip to content
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

[cts] stop device binaries build from silently failing #848

Closed
wants to merge 2 commits into from

Conversation

pbalcer
Copy link
Contributor

@pbalcer pbalcer commented Sep 5, 2023

Came up during a discussion in #840.

endif()

add_subdirectory(device_code)
add_subdirectory(kernel)
Copy link
Contributor

Choose a reason for hiding this comment

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

UR_DPCXX is only required for device_code stuff iirc, we could move all these out of the if altogether as long as we're content to build them without device code

@@ -13,7 +13,7 @@ macro(add_device_binary SOURCE_FILE)
COMMAND ${UR_DPCXX} -fsycl -fsycl-targets=${TRIPLE} -fsycl-device-code-split=off
${SOURCE_FILE} -o ${EXE_PATH}
COMMAND ${CMAKE_COMMAND} -E env SYCL_DUMP_IMAGES=true
${EXE_PATH} || (exit 0)
Copy link
Contributor

Choose a reason for hiding this comment

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

We're going to want a mechanism that filters which programs to build for which backends before doing this, as it stands I can build and run the program tests for e.g. l0 even though a number of the executables segfault presumably due to stuff like spec constants and images being unsupported. Possibly worth noting that despite the segfaults the IR still dumps. Maybe what we want instead for the short term is something to find libsycl at configure time so that particular common issue with these doesn't go unnoticed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's a good point... The build also fails right now because generating kernel_entry_points.h is part of the device_code cmake.

Approaching this from a different side, there are two 'build criteria' I see for kernel tests. First, as you've noted, ld needs to be able to find libsycl, and the second is having a device capable of running the kernels for the triplet.
If we can't rely on the binary failing, I think we need to detect those criteria prior to running it and emit a relevant message.

@pbalcer
Copy link
Contributor Author

pbalcer commented Sep 5, 2023

This approach doesn't work. Building kernel-related tests requires kernel_entry_points.h, which is created as part of creating the device binaries. We could generate this header separately, but I don't think it's worth it.
We also can't just fail the build if the application binaries fail, like I've done in this PR, because, as @aarongreig pointed out here, the application failing doesn't necessarily mean that the device binaries failed to build. We might want to a) verify build success by checking whether the device binary files exist and b) verify whether it's possible to generate binaries by checking for the existence of sycl and a suitable device for the target triplet.

I'm closing this for now and might revisit it later.

@pbalcer pbalcer closed this Sep 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants