fix(v4): rebuild backend_cuda_dsv4.o when the nvcc command changes - #1731
Merged
Merged
Conversation
The object is named after its source, but its contents come from $(NVCC) $(V4_NVCCFLAGS): -arch=$(CUDA_ARCH) (or the -gencode preset), -DCOLI_DSV4_NO_TC and the DeepGEMM defines. None of that is a prerequisite, so make decides on timestamps alone: asking for CUDA_ARCH=sm_80 after an sm_86 build recompiles nothing, exits 0, and the engine links the sm_86 object. Record the nvcc command in deepseek_v4.cudaflags and make backend_cuda_dsv4.o and deepseek-v4-clean depend on it. Same recipe-guarded stamp JustVugg#1707 added for $(CC) $(CFLAGS) (JustVugg#1702), kept in its own file so an arch change does not recompile the C units. The stamp is written by a recipe, so make -n writes nothing and clean removes it. Regression test mirrors JustVugg#1707's test_deepseek_v4_build_flags.py: stand-in nvcc, no toolchain, no GPU. Three of seven cases fail on dev; all seven pass with the fix, on GNU Make 3.81 and 4.3. Follows JustVugg#1707 / JustVugg#1702.
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.
backend_cuda_dsv4.ois named after its source, but what it contains comes from$(NVCC) $(V4_NVCCFLAGS):-arch=$(CUDA_ARCH)(or the-gencodepreset),-DCOLI_DSV4_NO_TC,and the DeepGEMM defines. None of that is a prerequisite, so make goes on timestamps alone:
The engine asked for sm_80 links the sm_86 object. This is the
CUDA_ARCH/backend_cuda_dsv4.ocase #1707 listed as out of scope. Follows #1707 / #1702.
Measured on
devata2e578c, the merge commit of #1707 — Ubuntu 24.04 container, real gcc 13.3,real nvcc 13.0,
cuobjdump, GNU Make 4.3, no GPU, no network, one tree throughout:cuobjdump --list-elf)a2e578ca2e578ca2e578cdeepseek-v4: CUDA build flags changed, rebuilding backend_cuda_dsv4.o, then the nvcc and link linesA genuinely up-to-date build prints nothing at all, in both trees, with and without
-j: bytecount 0, exit 0. So the
a2e578crows are the whole failure mode — the arch changes, make exitssuccessfully, and the engine silently links the object built for the previous target.
The same three failures reproduce on arm64 macOS with a stubbed nvcc and no toolkit at all.
make -nanddeepseek-v4-cleanwrite nothing, and the clean target removes the stamp whether ornot
CUDA=1is passed. Verified with a whole-tree manifest (path, mtime, sha256) taken before andafter each call, on GNU Make 4.3 (container) and GNU Make 3.81 (arm64 macOS, stubbed toolchain):
make -n deepseek-v4on a tree with no stamp leaves the stamp absent,make -n deepseek-v4-cleanleaves the tree byte-identical, and a real clean removes the stamp. The 3.81 run and its per-step
logs are kept with the rest of the evidence. This is a consequence of the stamp being written by a
recipe rather than at parse time.
The change
c/Makefile.deepseek-v4: record$(NVCC) $(V4_NVCCFLAGS)indeepseek_v4.cudaflagsand rewriteit only when it changes — the same recipe-guarded stamp fix(v4): rebuild the unit objects when the build flags change #1707 added for
$(CC) $(CFLAGS), kept inits own file so that an arch change does not recompile the C units.
backend_cuda_dsv4.odependson it, and
deepseek-v4-cleanremoves it..gitignore,c/tools/clean.py: the new stamp, like the other build artifacts.c/tests/test_deepseek_v4_cuda_arch_flags.py: same shape as fix(v4): rebuild the unit objects when the build flags change #1707'stest_deepseek_v4_build_flags.py— stand-in nvcc, sources copied to a temp dir with pinnedtimestamps, no toolchain and no GPU required. Seven tests:
CUDA_ARCH, the-gencodepreset,NO_TC, the "unchanged command does not rebuild" guard,make -nwriting nothing,cleanwriting nothing, and
cleanremoving the stamp. Againstdevthree of the seven fail (arch,preset,
NO_TC); with the patch all seven pass, on GNU Make 3.81 and 4.3.The parent
c/Makefilekeeps this kind of state for exactly this reason,CUDA_ARCHincluded, in.build-config(#306); this is the standalone build's equivalent, using the recipe-guarded formrather than
.build-config's parse-time$(file ...)write.Not covered by these runs:
make -C c cuda-teston a real GPU (no GPU is free here), Windows, andmacOS CUDA builds.
make checkon the patched tree is green on both hosts (Ubuntu 24.04 containerand arm64 macOS).
#1707's objects and stamp are untouched, so the two changes compose in eitherorder.
No runtime behaviour changes: the object is only rebuilt when the command that produces it moved.
Rollback: drop the patch, delete
c/deepseek_v4.cudaflags.