Repository navigation
Adding structural fingerprint mode as an alternative to hashing a merged print of a program - #5788
cuMikeBrown wants to merge 3 commits into
Conversation
…nt of the merged program. Signed-off-by: Michael Brown <michbrown@nvidia.com>
Command Bot: Processing... |
CI Summary (
|
| Job | Result |
|---|---|
binaries |
⏩ skipped |
build_and_test |
✅ success |
changes |
✅ success |
config_devdeps |
✅ success |
config_source_build |
⏩ skipped |
config_wheeldeps |
✅ success |
devdeps |
✅ success |
docker_image |
⏩ skipped |
documentation |
✅ success |
gen_code_coverage |
⏩ skipped |
metadata |
✅ success |
python_devel_wheel |
⏩ skipped |
python_metapackages |
⏩ skipped |
python_wheels |
⏩ skipped |
python_wheels_cu12_validation |
⏩ skipped |
python_wheels_cu13 |
⏩ skipped |
source_build |
⏩ skipped |
wheeldeps |
✅ success |
⏩ Skipped jobs (10) — intentionally skipped on PR builds; run on merge_group / workflow_dispatch
| Job |
|---|
binaries |
config_source_build |
docker_image |
gen_code_coverage |
python_devel_wheel |
python_metapackages |
python_wheels_cu12_validation |
python_wheels_cu13 |
python_wheels |
source_build |
All sub-jobs (47) — every matrix leg, with links
| Job | Status | Link |
|---|---|---|
| Build and test (amd64, gcc12, openmpi) / Dev environment (Debug) | ✅ success | view |
| Build and test (amd64, gcc12, openmpi) / Dev environment (Python) | ✅ success | view |
| Build and test (amd64, llvm, openmpi) / Dev environment (Debug) | ✅ success | view |
| Build and test (amd64, llvm, openmpi) / Dev environment (Python) | ✅ success | view |
| Build and test (arm64, llvm, openmpi) / Dev environment (Debug) | ✅ success | view |
| Build and test (arm64, llvm, openmpi) / Dev environment (Python) | ✅ success | view |
| CI Summary | ❔ in_progress | view |
| Check for stable CUDA-Q changes | ✅ success | view |
| Configure build (devdeps) | ✅ success | view |
| Configure build (source_build) | ⏩ skipped | view |
| Configure build (wheeldeps) | ✅ success | view |
| Create CUDA 12 Python wheels | ⏩ skipped | view |
| Create CUDA 13 Python wheels | ⏩ skipped | view |
| Create CUDA Quantum installer | ⏩ skipped | view |
| Create Docker images | ⏩ skipped | view |
| Create Python metapackages | ⏩ skipped | view |
| Create shared Python development wheel | ⏩ skipped | view |
| Documentation / Build and check docs | ✅ success | view |
| Gen code coverage | ⏩ skipped | view |
| Load dependencies (amd64, gcc12) / Caching | ✅ success | view |
| Load dependencies (amd64, gcc12) / Finalize | ✅ success | view |
| Load dependencies (amd64, gcc12) / Metadata | ✅ success | view |
| Load dependencies (amd64, llvm) / Caching | ✅ success | view |
| Load dependencies (amd64, llvm) / Finalize | ✅ success | view |
| Load dependencies (amd64, llvm) / Metadata | ✅ success | view |
| Load dependencies (arm64, gcc12) / Caching | ✅ success | view |
| Load dependencies (arm64, gcc12) / Finalize | ✅ success | view |
| Load dependencies (arm64, gcc12) / Metadata | ✅ success | view |
| Load dependencies (arm64, llvm) / Caching | ✅ success | view |
| Load dependencies (arm64, llvm) / Finalize | ✅ success | view |
| Load dependencies (arm64, llvm) / Metadata | ✅ success | view |
| Load source build cache | ⏩ skipped | view |
| Load wheel dependencies (amd64, 12.6) / Caching | ✅ success | view |
| Load wheel dependencies (amd64, 12.6) / Finalize | ✅ success | view |
| Load wheel dependencies (amd64, 12.6) / Metadata | ✅ success | view |
| Load wheel dependencies (amd64, 13.0) / Caching | ✅ success | view |
| Load wheel dependencies (amd64, 13.0) / Finalize | ✅ success | view |
| Load wheel dependencies (amd64, 13.0) / Metadata | ✅ success | view |
| Load wheel dependencies (arm64, 12.6) / Caching | ✅ success | view |
| Load wheel dependencies (arm64, 12.6) / Finalize | ✅ success | view |
| Load wheel dependencies (arm64, 12.6) / Metadata | ✅ success | view |
| Load wheel dependencies (arm64, 13.0) / Caching | ✅ success | view |
| Load wheel dependencies (arm64, 13.0) / Finalize | ✅ success | view |
| Load wheel dependencies (arm64, 13.0) / Metadata | ✅ success | view |
| Prepare cache clean-up | ❔ in_progress | view |
| Retrieve PR info | ✅ success | view |
| Validate CUDA 12 Python wheels | ⏩ skipped | view |
✅ Required checks (7/7) — declared in .github/required-checks.yml for push
| Required check | Status | Link |
|---|---|---|
| Build and test (amd64, llvm, openmpi) / Dev environment (Debug) | ✅ success | view |
| Build and test (amd64, llvm, openmpi) / Dev environment (Python) | ✅ success | view |
| Build and test (arm64, llvm, openmpi) / Dev environment (Debug) | ✅ success | view |
| Build and test (arm64, llvm, openmpi) / Dev environment (Python) | ✅ success | view |
| Build and test (amd64, gcc12, openmpi) / Dev environment (Debug) | ✅ success | view |
| Build and test (amd64, gcc12, openmpi) / Dev environment (Python) | ✅ success | view |
| Documentation / Build and check docs | ✅ success | view |
|
if we're convinced that the live IR hash doesn't miss anything, I'd be happy to switch over completely - no need to introduce I'll try to have a look later today. @khalatepradnya you might have a better sense of what to look out for. The risk with hashing is always to forget to hash a crucial bit of data and then get hash clashes all over the place. |
Signed-off-by: Michael Brown <michbrown@nvidia.com>
|
I'm confident this is the right direction, but not that nothing was missed, so would definitely want another set of eyes or alternatively a conservative approach: variable default to false for Nov release --> default to true after and for Mar release --> eliminate variable and old approach. In the interim, I've updated the tests so caching unit tests are forced with both modes (this actually runs caching tests 3 times, once with the mode set by the testing environment and then once with each mode forced). |
There was a problem hiding this comment.
Thanks, @cuMikeBrown for the changes!
This PR improves the fingerprint logic! My agent gathered following numbers-
Cache-hit launch latency on qpp-cpu, median of 3 runs of 300 launches:
| Kernel | Default | CUDAQ_FINGERPRINT_STRUCTURE=1 |
|---|---|---|
| Captured kernel, 10 gates | 0.54 ms | 0.17 ms |
| Captured kernel, 1,000 gates | 6.7 ms | 3.6 ms |
| Callable argument, 100 gates | 1.15 ms | 0.42 ms |
| Capture chain, depth 4 | 0.68 ms | 0.16 ms |
| Inline kernel, 1,000 gates (never fingerprints) | 3.0 ms | 3.1 ms |
Hashing one module is 12x to 140x faster than clone, print, and SHA-256 (1,339 ops: 3.2 ms against 0.14 ms). First compiles do not change measurably. Only kernels with captured kernels or callable arguments fingerprint, so only they benefit.
However, there are gaps w.r.t interop workflow and args substitution.
Reproducer: Compile both kernel files with nvq++ -fPIC --disable-mlir-links, and build the module the same way as python/tests/interop/test_cpp_quantum_algorithm_module.cpp.
// repro_lib.h
#include "cudaq/qis/qubit_qis.h"
namespace repro {
__qpu__ void repro_leaf(cudaq::qview<> qubits);
__qpu__ void repro_caller(cudaq::qview<> qubits);
} // namespace repro
// repro_leaf.cpp
#include "repro_lib.h"
__qpu__ void repro::repro_leaf(cudaq::qview<> qubits) { x(qubits[0]); }
// repro_caller.cpp
#include "repro_lib.h"
__qpu__ void repro::repro_caller(cudaq::qview<> qubits) {
repro_leaf(qubits);
x(qubits[1]);
}
// repro_module.cpp
#include "repro_lib.h"
#include "runtime/interop/PythonCppInterop.h"
#include <nanobind/nanobind.h>
NB_MODULE(repro_interop, m) {
cudaq::python::addDeviceKernelInterop<cudaq::qview<>>(m, "rq", "repro_caller", "");
cudaq::python::addDeviceKernelInterop<cudaq::qview<>>(m, "rq", "repro_leaf", "");
}import cudaq
from repro_interop import rq
@cudaq.kernel
def caller() -> bool:
q = cudaq.qvector(2)
rq.repro_caller(q)
return mz(q[0])
print(caller())rq.repro_leaf only (control) |
rq.repro_caller |
|
|---|---|---|
| main | True | True |
| this PR, default | True | True |
| this PR, structural | True | JIT session error: Symbols not found, abort (exit code 134) |
| this PR, structural, with comments 1 to 5 applied | True | True |
| if (hasUnownedDependency) | ||
| return std::nullopt; |
There was a problem hiding this comment.
A closure can bring in a declaration that the C++ registry defines. On that path, this function returns without a module, so pyLaunchModule compiles an unmerged mod.clone(). Compiler::prepareModule fills registry definitions (lowerQuakeCodeBuildModule) before it merges closures, so the definition never arrives, and the JIT aborts with Symbols not found (exit code 134).
| if (hasUnownedDependency) | |
| return std::nullopt; | |
| if (hasUnownedDependency) { | |
| resolvedModule = mod.clone(); | |
| cudaq_internal::compiler::mergeAllCallableClosures(resolvedModule.get(), | |
| name, rawArgs); | |
| return std::nullopt; | |
| } |
| /// Fingerprint by describing the resolved program rather than building it. | ||
| /// \p mod is left untouched, so the caller gets no clone to compile. | ||
| std::optional<std::array<std::uint8_t, 32>> | ||
| fingerprintByStructure(const std::string &name, mlir::ModuleOp mod, | ||
| const std::vector<void *> &rawArgs) { |
There was a problem hiding this comment.
Part of the fix at line 340: thread resolvedModule through, and say when it is filled.
| /// Fingerprint by describing the resolved program rather than building it. | |
| /// \p mod is left untouched, so the caller gets no clone to compile. | |
| std::optional<std::array<std::uint8_t, 32>> | |
| fingerprintByStructure(const std::string &name, mlir::ModuleOp mod, | |
| const std::vector<void *> &rawArgs) { | |
| /// Fingerprint by describing the resolved program rather than building it. | |
| /// A cacheable program leaves \p resolvedModule empty. A program that cannot | |
| /// be cached gets the merged clone, because compiling the unmerged module | |
| /// would miss registry definitions that its closures declare. | |
| std::optional<std::array<std::uint8_t, 32>> | |
| fingerprintByStructure(const std::string &name, mlir::ModuleOp mod, | |
| const std::vector<void *> &rawArgs, | |
| mlir::OwningOpRef<mlir::ModuleOp> &resolvedModule) { |
| if (useStructuralFingerprint()) | ||
| return fingerprintByStructure(name, mod, rawArgs); |
There was a problem hiding this comment.
Part of the fix at line 340.
| if (useStructuralFingerprint()) | |
| return fingerprintByStructure(name, mod, rawArgs); | |
| if (useStructuralFingerprint()) | |
| return fingerprintByStructure(name, mod, rawArgs, resolvedModule); |
| std::optional<std::array<std::uint8_t, 32>> | ||
| fingerprintByStructure(const std::string &name, mlir::ModuleOp mod, | ||
| const std::vector<void *> &rawArgs); |
There was a problem hiding this comment.
Part of the fix at line 340.
| std::optional<std::array<std::uint8_t, 32>> | |
| fingerprintByStructure(const std::string &name, mlir::ModuleOp mod, | |
| const std::vector<void *> &rawArgs); | |
| std::optional<std::array<std::uint8_t, 32>> | |
| fingerprintByStructure(const std::string &name, mlir::ModuleOp mod, | |
| const std::vector<void *> &rawArgs, | |
| mlir::OwningOpRef<mlir::ModuleOp> &resolvedModule); |
| /// Setting `CUDAQ_FINGERPRINT_STRUCTURE` summarizes the module's structure | ||
| /// instead of printing it, which skips the clone, the merge and the printing | ||
| /// but leaves \p resolvedModule empty, so a caller that misses the cache must | ||
| /// clone \p mod itself. Both modes answer the same question, but their digests | ||
| /// are not interchangeable, and the structural digest is valid only within the | ||
| /// process that computed it, since it mixes in the identity of | ||
| /// context-uniqued attributes and types. |
There was a problem hiding this comment.
With the fix at ProgramFingerprint.cpp line 340, a program that cannot be cached gets the merged clone in both modes.
| /// Setting `CUDAQ_FINGERPRINT_STRUCTURE` summarizes the module's structure | |
| /// instead of printing it, which skips the clone, the merge and the printing | |
| /// but leaves \p resolvedModule empty, so a caller that misses the cache must | |
| /// clone \p mod itself. Both modes answer the same question, but their digests | |
| /// are not interchangeable, and the structural digest is valid only within the | |
| /// process that computed it, since it mixes in the identity of | |
| /// context-uniqued attributes and types. | |
| /// Setting `CUDAQ_FINGERPRINT_STRUCTURE` summarizes the module's structure | |
| /// instead of printing it, which skips the clone, the merge and the printing. | |
| /// A cacheable program then leaves \p resolvedModule empty, so a caller that | |
| /// misses the cache must clone \p mod itself. A program that cannot be cached | |
| /// still gets the merged clone. Both modes answer the same question, but their | |
| /// digests are not interchangeable, and the structural digest is valid only | |
| /// within the process that computed it, since it mixes in the identity of | |
| /// context-uniqued attributes and types. |
| // Number results before operands are mixed in, so a value used by a | ||
| // later operation is already numbered when that use is hashed. | ||
| for (auto result : op->getResults()) | ||
| number(result.getAsOpaquePointer()); |
There was a problem hiding this comment.
A result's number is assigned here but mixed in only at its uses. Block order is free in a CFG region, so a block can use values defined in a block listed after it. Swapping two such operands then changes the program but not the digest. These two functions get the same structural digest and different print digests:
func.func @k() {
%q = quake.alloca !quake.ref
cf.br ^bb2
^bb1:
quake.rx (%a) %q : (f64, !quake.ref) -> () // swap %a and %b: same digest
quake.ry (%b) %q : (f64, !quake.ref) -> ()
return
^bb2:
%a = arith.constant 0.5 : f64
%b = arith.constant 1.5 : f64
cf.br ^bb1
}| // Number results before operands are mixed in, so a value used by a | |
| // later operation is already numbered when that use is hashed. | |
| for (auto result : op->getResults()) | |
| number(result.getAsOpaquePointer()); | |
| // Mix each result's number at its definition too. Block order is free in | |
| // a CFG region, so a use can be visited before its definition. | |
| for (auto result : op->getResults()) | |
| mix(number(result.getAsOpaquePointer())); |
| for (auto &block : region) { | ||
| mix(number(&block)); | ||
| mix(block.getNumArguments()); |
There was a problem hiding this comment.
Nothing marks where a block's operations end, because the walk visits nested regions inline. Terminators usually disambiguate, but cc.arg_subst has no terminator, and every substitution module hashed at line 377 contains it. These two modules hash the same, yet one substitutes 7 and the other 3:
module {
cc.arg_subst[0] {
%0 = arith.constant 3 : i64
%1 = arith.constant 7 : i64
}
}module {
cc.arg_subst[0] {
%0 = arith.constant 3 : i64
}
%1 = arith.constant 7 : i64
}ArgumentConverter never emits the second shape today, so this is latent. Mixing in the operation count closes it:
| for (auto &block : region) { | |
| mix(number(&block)); | |
| mix(block.getNumArguments()); | |
| for (auto &block : region) { | |
| mix(number(&block)); | |
| mix(block.getOperations().size()); | |
| mix(block.getNumArguments()); |
Given the significant change, this has been done as opt-in via the bool
CUDAQ_FINGERPRINT_STRUCTURE, with the default set to false so there is no change in default logic.Current fingerprinting does a merged print of the program to generate a unique hash. The functional equivalent added here instead walks the IR graph with minimal hashing for a unique id. It should be significantly faster in both the case of a cache hit or a new compile (although time in the latter is insignificant).