Check generated device code with FileCheck.jl - #783
Merged
Merged
Conversation
The test suite covers what kernels compute, but nothing pins down how they compile. Several of the guarantees KernelAbstractions makes are about the generated code rather than the result -- `@inbounds` removing the out-of-bounds path, a statically sized ndrange folding `__validindex` away, `@localmem` landing in the workgroup address space, `@atomic` becoming a native `atomicrmw` -- and all of those can regress silently. Add test/codegen.jl, which states them as LLVM FileCheck directives over `@device_code_llvm` output, using FileCheck.jl. The tests live outside `Testsuite`: that module is included by the backend packages, which cannot pick up new test dependencies, and the patterns are specific to the in-tree POCL/SPIR-V back-end anyway. They avoid pointer syntax so they hold for both the typed pointers of LLVM 15 (Julia 1.10) and the opaque pointers of later versions. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DbbtGLjxXpt7kZj6h2yVdK
Collaborator
|
Also #754 (review)? |
Contributor
Benchmark ResultsShow table
Benchmark PlotsA plot of the benchmark results have been uploaded as an artifact to the workflow run for this PR. |
`Pkg.test` defaults to `--check-bounds=yes`, which forces bounds checks on regardless of `@inbounds`. Two of the checks assert that `@inbounds` removes the out-of-bounds path, which under that flag is not merely untestable but false, so CI failed everywhere while a direct `julia test/runtests.jl` passed. Move the checks to test/codegen_checks.jl and run them from a subprocess that inherits neither `--check-bounds` nor `--code-coverage`, following the existing pattern in test/coverage.jl. Coverage instruments the very kernels being inspected, and under `Pkg.test(coverage=true)` would also fold the subprocess into the outer report. codegen_checks.jl refuses to run unless bounds checking is on `auto`, so a future regression in the flag handling fails loudly rather than quietly asserting nothing. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DbbtGLjxXpt7kZj6h2yVdK
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #783 +/- ##
==========================================
+ Coverage 0.70% 66.09% +65.39%
==========================================
Files 21 23 +2
Lines 1839 1991 +152
==========================================
+ Hits 13 1316 +1303
+ Misses 1826 675 -1151 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
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.
The test suite covers what kernels compute, but nothing pins down how they compile. Several of the guarantees KernelAbstractions makes are about the generated code rather than the result, and all of them can regress silently today:
@inboundsthe kernel carries the out-of-bounds path (the errorprintfplusgpu_report_exception/gpu_signal_exception), and@inboundsremoves all of it;__validindexfolds away and the kernel body becomes straight-line code (paired with a dynamic-ndrange kernel that still branches, so the check is not vacuous);@localmembecomes a module-level allocation in address space 3 that the kernel reads and writes directly;@synchronizebecomes a SPIR-V control barrier;@atomicbecomes a nativeatomicrmw, not a compare-and-swap loop;@printbecomes a single variadicprintfcall, not one call per argument.test/codegen_checks.jlstates each of those as LLVM FileCheck directives over@device_code_llvmoutput, using FileCheck.jl.test/codegen.jlruns it.Why a subprocess
Pkg.testdefaults to--check-bounds=yes, which forces bounds checks on regardless of@inbounds. Under it the two@inboundsassertions are not merely untestable but false. Making them conditional would mean they never run in CI, so insteadtest/codegen.jlruns the checks in a subprocess that inherits neither--check-boundsnor--code-coverage, following the existing pattern intest/coverage.jl— coverage instruments the very kernels being inspected, and underPkg.test(coverage=true)would also fold the subprocess into the outer report.codegen_checks.jlrefuses to run unless bounds checking is onauto, so a future regression in the flag handling fails loudly rather than quietly asserting nothing.To iterate on a pattern, run it directly:
Notes
Testsuite. That module is included by the backend packages, which cannot pick up new test dependencies, and the patterns are specific to the in-tree POCL/SPIR-V back-end anyway.implicit_check_notrather than a trailing@check_not: aCHECK-NOTafter the last positive check only covers the range from that match to the end of the input, which forret voidis nothing at all.@localmemaccesses are@check_dag, because LLVM is free to emit the basic blocks in any order — it does in fact sink the barrier block above them here.FileCheckv1.2 depends only onLLVM_utils_unified_jll, a FileCheck-only artifact, rather than on a full LLVM build.Testing
Pkg.test(coverage=true)passes locally, and CI is green across the matrix.Each check was verified non-vacuous by mutating the kernel it covers — dropping
@inbounds,@synchronizeand@atomiceach fail exactly the corresponding test. Breaking a pattern was also verified to fail the parent testset, with the subprocess's annotated FileCheck output surfaced in the log.🤖 Generated with Claude Code
https://claude.ai/code/session_01DbbtGLjxXpt7kZj6h2yVdK