Stop emitting aliasscope regions in @kernel - #785
Merged
Merged
Conversation
`transform_gpu!` wrapped every kernel body in `Expr(:aliasscope)` / `Expr(:popaliasscope)`. Those tokens only do something when a load inside the region is tied to them, i.e. Base's `Const(::Array)` lowering, which emits `!alias.scope`/`!noalias` against the enclosing scope. That was the native-Julia CPU backend, replaced by POCL in #556. No current backend takes that path: POCL's `@Const` goes through `const_arrayref` on `CLDeviceArray` (`!invariant.load`), and the external GPU backends use their own device-array `Const` wrappers over `ldg`/`!invariant.load`. The region was therefore inert, while still forcing the body into a scope that blocks some code motion and showing up in the lowered IR. Drop the `ConstAdaptor`/`Array` rule along with it. `constify` runs inside the kernel, after arguments have been adapted to device arrays, so it was already unreachable -- and without an enclosing aliasscope region the resulting `Const` wrapper lowered to ordinary loads anyway. Assisted-by: Claude Code (Opus 5)
Contributor
Benchmark ResultsShow table
Benchmark PlotsA plot of the benchmark results have been uploaded as an artifact to the workflow run for this PR. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #785 +/- ##
==========================================
+ Coverage 0.70% 65.91% +65.20%
==========================================
Files 21 23 +2
Lines 1839 1998 +159
==========================================
+ Hits 13 1317 +1304
+ Misses 1826 681 -1145 ☔ 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
1 participant
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.
transform_gpu!wrapped every kernel body inExpr(:aliasscope)/Expr(:popaliasscope). Those tokens only do something when a load inside the region is tied to them — i.e. Base'sConst(::Array)lowering, which emits!alias.scope/!noaliasagainst the enclosing scope. That consumer was the native-Julia CPU backend, replaced by POCL in #556.No current backend takes that path:
CPU():@Constgoes throughconst_arrayrefonCLDeviceArray, emitting!invariant.load.Constwrappers overldg/!invariant.load.So the region was inert, while still forcing the body into a scope that blocks some code motion and showing up in the lowered IR.
docs/src/design.mdwas already updated to say!invariant.loadon POCL when the CPU backend was swapped out; the macro just wasn't.This also drops
Adapt.adapt_storage(::ConstAdaptor, a::Array).constifyruns inside the kernel, after arguments have been adapted to device arrays, so it was already unreachable — and without an enclosing aliasscope region the resultingConstwrapper lowered to ordinary loads anyway.ConstAdaptoritself stays; it's the dispatch tag backends register against.Testing
Full suite on the POCL backend: 2481 passed, 0 failed, 2 errored. The 2 errors are
ifelse on aggregate types(ComplexF32/ComplexF64, #760) failing to translate to SPIR-V; they reproduce identically on unmodifiedmain, and the pass/error counts are unchanged from baseline.🤖 Generated with Claude Code