Skip to content

improve sample look and feel consistency - #176

Open
bashbaug wants to merge 4 commits into
mainfrom
consistent-look-and-feel
Open

improve sample look and feel consistency#176
bashbaug wants to merge 4 commits into
mainfrom
consistent-look-and-feel

Conversation

@bashbaug

Copy link
Copy Markdown
Owner

fixes #174

fixes #175

Move code to setup the platform and device to a shared utility
function. Add support for verbose setup, which will print more
information about the platform and device than the default.
Very few samples need the OpenCL platform, and when they do, they
can query it from the device.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 Changes recommended

The new device/platform setup utilities introduce correctness/build issues (negative index handling and std::move usage without <utility>), and one sample duplicates platform enumeration unnecessarily.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR standardizes device/platform selection and runtime output across the sample suite by introducing a shared utility for device setup (including optional verbose device details) and updating many samples to use it.

Changes:

  • Add checkDeviceIndex() and setupDevice() utilities to validate indices and print consistent platform/device information (with a verbose mode).
  • Update many OpenCL/OpenGL/Vulkan/USM/SVM samples to use setupDevice() and add a --verbose CLI switch.
  • Normalize main() signatures and simplify OpenGL context creation to derive the platform from the selected device.
File summaries
File Description
include/util.hpp Adds device index validation + centralized platform/device setup/printing helper.
tutorials/interceptlayer/main.cpp Normalizes main() signature formatting.
tutorials/interceptlayer/main-start.cpp Normalizes main() signature formatting.
tutorials/interceptlayer/main-part1solution.cpp Normalizes main() signature formatting.
tutorials/interceptlayer/main-part2solution.cpp Normalizes main() signature formatting.
tutorials/interceptlayer/main-part3solution.cpp Normalizes main() signature formatting.
tutorials/interceptlayer/main-part4solution.cpp Normalizes main() signature formatting.
tutorials/interceptlayer/main-part5solution.cpp Normalizes main() signature formatting.
samples/vulkan/00_juliavk/main.cpp Uses setupDevice() + adds --verbose; uses device-derived platform for extension lookups.
samples/vulkan/01_nbodyvk/main.cpp Uses setupDevice() + adds --verbose; uses device-derived platform for extension lookups.
samples/opengl/00_juliagl/main.cpp Uses setupDevice() + adds --verbose; derives platform from device in GL-sharing context creation.
samples/opengl/01_nbodygl/main.cpp Uses setupDevice() + adds --verbose and standardizes context/queue creation.
samples/opengl/02_sobelgl/main.cpp Uses setupDevice() + adds --verbose; derives platform from device in GL-sharing context creation.
samples/images/00_enumimageformats/main.cpp Uses setupDevice() + adds --verbose for consistent platform/device output.
samples/usm/00_usmqueries/main.cpp Normalizes main() signature formatting.
samples/usm/01_usmmeminfo/main.cpp Uses setupDevice() + adds --verbose; updates device queries to use selected device.
samples/usm/100_dmemhelloworld/main.cpp Uses setupDevice() + adds --verbose; updates USM allocation calls to selected device.
samples/usm/101_dmemlinkedlist/main.cpp Uses setupDevice() + adds --verbose; updates init/device usage.
samples/usm/200_hmemhelloworld/main.cpp Uses setupDevice() + adds --verbose for consistent platform/device output.
samples/usm/201_hmemlinkedlist/main.cpp Uses setupDevice() + adds --verbose; updates init/device usage.
samples/usm/300_smemhelloworld/main.cpp Uses setupDevice() + adds --verbose; updates USM allocation calls to selected device.
samples/usm/301_smemlinkedlist/main.cpp Uses setupDevice() + adds --verbose; updates init/device usage.
samples/usm/310_usmmigratemem/main.cpp Uses setupDevice() + adds --verbose; updates USM allocation calls to selected device.
samples/usm/400_sysmemhelloworld/main.cpp Uses setupDevice() + adds --verbose; updates device queries/queue creation.
samples/svm/00_svmqueries/main.cpp Normalizes main() signature formatting.
samples/svm/100_cgsvmhelloworld/main.cpp Uses setupDevice() + adds --verbose; standardizes device capability queries.
samples/svm/101_cgsvmlinkedlist/main.cpp Uses setupDevice() + adds --verbose; standardizes device capability queries and init.
samples/svm/200_fgsvmhelloworld/main.cpp Uses setupDevice() + adds --verbose; standardizes device capability queries.
samples/svm/201_fgsvmlinkedlist/main.cpp Uses setupDevice() + adds --verbose; standardizes device capability queries and init.
samples/00_enumopencl/main.cpp Normalizes main() signature formatting.
samples/00_enumopenclpp/main.cpp Normalizes main() signature formatting.
samples/00_enumqueuefamilies/main.cpp Normalizes main() signature formatting.
samples/00_extendeddevicequeries/main.cpp Normalizes main() signature formatting.
samples/00_loaderinfo/main.cpp Normalizes main() signature formatting.
samples/00_newqueries/main.cpp Normalizes main() signature formatting.
samples/00_newqueriespp/main.cpp Normalizes main() signature formatting.
samples/00_spirvqueries/main.cpp Normalizes main() signature formatting.
samples/01_copybuffer/main.cpp Uses setupDevice() + adds --verbose; standardizes context/queue creation.
samples/02_copybufferkernel/main.cpp Uses setupDevice() + adds --verbose; standardizes context/queue creation.
samples/03_mandelbrot/main.cpp Uses setupDevice() + adds --verbose; standardizes context/queue creation.
samples/04_julia/main.cpp Uses setupDevice() + adds --verbose; standardizes context/queue creation.
samples/04_sobel/main.cpp Uses setupDevice() + adds --verbose; standardizes context/queue creation.
samples/05_kernelfromfile/main.cpp Uses setupDevice() + adds --verbose; standardizes context/queue creation.
samples/05_spirvkernelfromfile/main.cpp Uses setupDevice() + adds --verbose; standardizes context/queue creation and device capability checks.
samples/06_ndrangekernelfromfile/main.cpp Uses setupDevice() + adds --verbose; standardizes context/queue creation.
samples/10_queueexperiments/main.cpp Uses setupDevice() + adds --verbose; standardizes device/context selection.
samples/11_semaphores/main.cpp Uses setupDevice() + adds --verbose; updates device queries/queues accordingly.
samples/12_commandbuffers/main.cpp Uses setupDevice() + adds --verbose; updates device info queries and queue creation.
samples/12_commandbufferspp/main.cpp Uses setupDevice() + adds --verbose; updates device info queries and queue creation.
samples/13_mutablecommandbuffers/main.cpp Uses setupDevice() + adds --verbose; updates device queries and queue creation.
samples/14_ooqcommandbuffers/main.cpp Uses setupDevice() + adds --verbose; updates device queries and queue creation.
samples/15_mutablecommandbufferasserts/main.cpp Uses setupDevice() + adds --verbose; updates device queries and queue creation.
samples/16_floatatomics/main.cpp Uses setupDevice() + adds --verbose; updates extension/capability queries and queue creation.
samples/20_matrixexperiments-bf16/main.cpp Uses setupDevice() + adds --verbose; centralizes device selection/printing.
samples/20_matrixexperiments-i8/main.cpp Uses setupDevice() + adds --verbose; centralizes device selection/printing.
samples/20_matrixexperiments-tf32/main.cpp Uses setupDevice() + adds --verbose; centralizes device selection/printing.
Review details

Suppressed comments (1)

include/util.hpp:114

  • checkDeviceIndex() has the same issue as checkPlatformIndex() had previously: it doesn’t reject negative indices, so a -1 device index can later be used for indexing and cause undefined behavior.
static bool checkDeviceIndex(
    const std::vector<cl::Device>& devices,
    int deviceIndex)
{
    if (devices.size() == 0) {
  • Files reviewed: 56/56 changed files
  • Comments generated: 3
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread include/util.hpp
Comment thread include/util.hpp Outdated
Comment thread samples/11_semaphores/main.cpp

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔵 Needs a closer look

It refactors OpenCL platform/device selection and output behavior across a large number of samples and introduces a new shared helper, which is difficult to fully validate for build/runtime correctness without running the full sample set.

Review details

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

samples/11_semaphores/main.cpp:82

  • The variable name has_cl_khr_command_buffer is misleading here because it actually tracks support for CL_KHR_SEMAPHORE_EXTENSION_NAME, which makes the subsequent logic harder to read and maintain.
    bool has_cl_khr_command_buffer =
        checkDeviceForExtension(device, CL_KHR_SEMAPHORE_EXTENSION_NAME);
    if (has_cl_khr_command_buffer) {
        printf("Device supports " CL_KHR_SEMAPHORE_EXTENSION_NAME ".\n");
  • Files reviewed: 56/56 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

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.

add a utility function to print device information add a utility function to check the device index

2 participants