fix(ggml-cuda): require nonzero graph ID for property bypass - #676
Open
cheese-cakee wants to merge 2 commits into
Open
fix(ggml-cuda): require nonzero graph ID for property bypass#676cheese-cakee wants to merge 2 commits into
cheese-cakee wants to merge 2 commits into
Conversation
Contributor
There was a problem hiding this comment.
All reported issues were addressed across 4 files
You’re at about 90% of the monthly reviewed-line limit. You may want to disable incremental reviews to conserve quota. Reviews will continue until that limit is exceeded. If you need help avoiding interruptions, please contact contact@cubic.dev.
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Prevent a direct GGML graph from replaying an older graph's CUDA/HIP executable when its metadata arena is reused and forced property-check bypass is requested.
Direct graphs have an unset generation ID (
uid = 0). The old bypass condition accepted that unset ID as a match. Rebuilding a different operation at the same first-node address could therefore replay the old operation: the regression changesNEGtoSQR, but the old libraries return-3instead of9for input3.The bypass now requires a matching, nonzero generation ID. Zero-ID graphs retain ordinary property inspection, capture, and replay. This is a correctness fix, not a throughput claim.
Changes
ggml-cuda.cuand document the requirement inggml-cuda.h.Verification
Base:
5c775e31dc700ed2d5fd9dabbe1cb20bd083d949(current main when prepared).Runtime fix: standalone hardware validation
The pre-publication validation report records the following results for the same runtime fix shipped in this PR. Adding the registered regression test and CTest wiring did not change the production predicate or invalidate these earlier hardware results.
sm_89-3instead of99400gfx1201-3instead of99400gfx1151-3instead of99400Normal property inspection also produced the correct result on all three targets. The HIP runs used ROCm 7.2.4 / HIP 7.2.53211. These are the recorded standalone-validation results, not a claim that the later CTest packaging was rerun on HIP.
Registered regression: additional CUDA validation
Fresh local validation: RTX 4050 Laptop (
sm_89), WSL Ubuntu, CUDA 12.6, Release build.-3instead of9, exit 134git diff --checkReproduce with a graph-enabled server build:
For HIP, configure with
-DDFLASH27B_GPU_BACKEND=hip -DGGML_HIP_GRAPHS=ON.