From 19f3d5dca0d4bb54a19fece35ef1ac5798fcc5ea Mon Sep 17 00:00:00 2001 From: Gleb Alekseev Date: Sat, 8 Aug 2026 23:41:13 +0200 Subject: [PATCH] Give devices without PCI topology a stable cache identity The kernel cache never hit on an Apple GPU. getUniqueDeviceIdentifier asks for CL_DEVICE_PCI_BUS_ID_NV and CL_DEVICE_PCI_SLOT_ID_NV, which Apple's runtime does not implement, and clGetWrapper returned the value it declared without initializing it and without looking at the error code. The identifier was therefore whatever happened to be on the stack: three consecutive runs on an M4 Max wrote three cache files and every one of them reported "precompiled = no". Queries now go through clGetWrapperChecked, which zeroes the value and reports whether the runtime answered. Where neither vendor extension is available the identifier is an FNV-1a hash of the device name, vendor and driver version. Two identical GPUs hash to one entry, which is what you want: the binary they need is the same. Devices that do report PCI topology keep the identifier they had, so caches already on disk stay valid. The same missing check made clGetWrapperString size its buffer from an uninitialized length whenever a query failed; it now returns an empty string. Also drops the container smoke test's blind spot: without a GPU the program exits before loading a kernel, so nothing verified the kernels were in the image at all, and points the two bench examples at pr/57 rather than a branch that only ever existed on one laptop. Co-authored-by: Cursor --- .github/workflows/docker.yml | 5 ++++ bench/README.md | 2 +- bench/prepare-native.sh | 2 +- src/profanity.cpp | 53 +++++++++++++++++++++++++++++------- 4 files changed, 50 insertions(+), 12 deletions(-) diff --git a/.github/workflows/docker.yml b/.github/workflows/docker.yml index 63908b5..20458f7 100644 --- a/.github/workflows/docker.yml +++ b/.github/workflows/docker.yml @@ -37,6 +37,11 @@ jobs: run: | docker run --rm profanity2:ci --help | grep -q '^usage: ' docker run --rm profanity2:ci clinfo --version + # Without a GPU the program exits before it ever loads a kernel, so + # nothing else here would notice the kernels missing from the image. + # They have to sit next to the binary, which is where it looks first. + docker run --rm profanity2:ci ls /opt/profanity2/profanity2.x64 \ + /opt/profanity2/keccak.cl /opt/profanity2/profanity.cl # No GPU on the runner, so the entrypoint must refuse to start. ! docker run --rm profanity2:ci --benchmark -z "$(printf 'a%.0s' {1..128})" diff --git a/bench/README.md b/bench/README.md index 1dcd894..72379e0 100644 --- a/bench/README.md +++ b/bench/README.md @@ -107,7 +107,7 @@ The image is built for `linux/amd64` by default because the Linux branch of the On macOS a container is a Linux virtual machine, and the Apple GPU is not passed into it: there is no counterpart to the NVIDIA Container Toolkit, the Ubuntu base image cannot reach `OpenCL.framework` on the host, and this directory's Dockerfile registers the NVIDIA ICD in any case. Rebuilding the image for `linux/arm64` changes the CPU architecture and nothing else, so a container on a Mac measures at best a CPU OpenCL runtime. To compare two revisions on an Apple GPU, build them on the host: ```bash -WORK=$(bench/prepare-native.sh 9011bcd pr57-head) +WORK=$(bench/prepare-native.sh 9011bcd pr/57) BENCH_ROOT=$WORK bench/run-benchmark.sh --mode leading --repeats 3 ``` diff --git a/bench/prepare-native.sh b/bench/prepare-native.sh index 88309e9..0fbe7f3 100755 --- a/bench/prepare-native.sh +++ b/bench/prepare-native.sh @@ -26,7 +26,7 @@ # # The workdir is printed on the last line, so the two steps compose: # -# WORK=$(bench/prepare-native.sh 9011bcd pr57-head) +# WORK=$(bench/prepare-native.sh 9011bcd pr/57) # BENCH_ROOT=$WORK bench/run-benchmark.sh set -euo pipefail diff --git a/src/profanity.cpp b/src/profanity.cpp index 5cae385..245388b 100644 --- a/src/profanity.cpp +++ b/src/profanity.cpp @@ -147,18 +147,31 @@ std::vector getAllDevices(cl_device_type deviceType = CL_DEVICE_TY return vDevices; } +// Reads a fixed-size device or program property, reporting whether the runtime +// actually answered. Everything below asks for at least one property that not +// every runtime implements, and the value has to be zeroed rather than left as +// whatever was on the stack when the query fails. +template +bool clGetWrapperChecked(T & t, U function, V param, W param2) { + t = T(); + return function(param, param2, sizeof(t), &t, NULL) == CL_SUCCESS; +} + template T clGetWrapper(U function, V param, W param2) { T t; - function(param, param2, sizeof(t), &t, NULL); + clGetWrapperChecked(t, function, param, param2); return t; } template std::string clGetWrapperString(U function, V param, W param2) { - size_t len; - function(param, param2, 0, NULL, &len); - char * const szString = new char[len]; + size_t len = 0; + if (function(param, param2, 0, NULL, &len) != CL_SUCCESS || len == 0) { + return std::string(); + } + + char * const szString = new char[len](); function(param, param2, len, szString, NULL); std::string r(szString); delete[] szString; @@ -167,7 +180,7 @@ std::string clGetWrapperString(U function, V param, W param2) { template std::vector clGetWrapperVector(U function, V param, W param2) { - size_t len; + size_t len = 0; function(param, param2, 0, NULL, &len); len /= sizeof(T); std::vector v; @@ -209,14 +222,34 @@ unsigned int getUniqueDeviceIdentifier(const cl_device_id & deviceId) { // cl_device_topology_amd struct and the TYPE_PCIE constant, hence the // second condition. #if defined(CL_DEVICE_TOPOLOGY_AMD) && defined(CL_DEVICE_TOPOLOGY_TYPE_PCIE_AMD) - auto topology = clGetWrapper(clGetDeviceInfo, deviceId, CL_DEVICE_TOPOLOGY_AMD); - if (topology.raw.type == CL_DEVICE_TOPOLOGY_TYPE_PCIE_AMD) { + cl_device_topology_amd topology; + if (clGetWrapperChecked(topology, clGetDeviceInfo, deviceId, CL_DEVICE_TOPOLOGY_AMD) + && topology.raw.type == CL_DEVICE_TOPOLOGY_TYPE_PCIE_AMD) { return (topology.pcie.bus << 16) + (topology.pcie.device << 8) + topology.pcie.function; } #endif - cl_int bus_id = clGetWrapper(clGetDeviceInfo, deviceId, CL_DEVICE_PCI_BUS_ID_NV); - cl_int slot_id = clGetWrapper(clGetDeviceInfo, deviceId, CL_DEVICE_PCI_SLOT_ID_NV); - return (bus_id << 16) + slot_id; + cl_int bus_id, slot_id; + if (clGetWrapperChecked(bus_id, clGetDeviceInfo, deviceId, CL_DEVICE_PCI_BUS_ID_NV) + && clGetWrapperChecked(slot_id, clGetDeviceInfo, deviceId, CL_DEVICE_PCI_SLOT_ID_NV)) { + return (bus_id << 16) + slot_id; + } + + // Neither vendor's PCI extension is available, which is where Apple's + // runtime ends up. What identifies a device here is what it calls itself, + // and that is enough for a compiled kernel: two identical GPUs hashing to + // one entry is correct, because the binary they need is the same. + const std::string strIdentity = + clGetWrapperString(clGetDeviceInfo, deviceId, CL_DEVICE_NAME) + "\n" + + clGetWrapperString(clGetDeviceInfo, deviceId, CL_DEVICE_VENDOR) + "\n" + + clGetWrapperString(clGetDeviceInfo, deviceId, CL_DRIVER_VERSION); + + // FNV-1a. It only has to be stable from one run to the next. + unsigned int hash = 2166136261u; + for (const char c : strIdentity) { + hash = (hash ^ static_cast(c)) * 16777619u; + } + + return hash; } template bool printResult(const T & t, const cl_int & err) {