Repository navigation
Inline the generated functions that return their body explicitly - #951
Merged
Merged
Conversation
`@inline` on a generated function only marks the body for inlining when the generator falls through to it, not when it returns it explicitly, as Runic formats it. Move the `inline_body` helper that the shuffles use to the utilities, and use it for `argument_types` too.
`@inline @generated` functions whose generator returns the body with an
explicit `return`, which Runic inserts, were not marked for inlining.
On the POCL back-end that includes `arrayref_union` and
`arrayset_union`, so that loads from arrays of isbits unions became
calls returning the element through a stack slot plus a pointer to it.
This is slower, and in some kernels the LLVM SPIR-V back-end then emits
invalid SPIR-V for phis of these pointers (e.g. AcceleratedKernels'
reductions with a `Union{Missing, Float32}` accumulator).
Mark the bodies for inlining with `KernelInterface.inline_body`, here
and in the host-side generated functions with the same problem.
Contributor
Benchmark resultsCompared with a13691a5b, the merge base.
All benchmarks
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #951 +/- ##
==========================================
- Coverage 84.91% 84.11% -0.81%
==========================================
Files 31 31
Lines 2612 2562 -50
==========================================
- Hits 2218 2155 -63
- Misses 394 407 +13 ☔ 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.
@inlinehad no effect on most of our@inline @generatedfunctions. Julia marks a generated body for inlining by wrapping what the generator returns, and an explicitreturnin the generator skips that wrapper. Runic adds thatreturnto every generator:Only KernelInterface's shuffles were inlined, because they already wrap their bodies with
inline_body. Everything else that was meant to be inlined wasn't:arrayref_unionandarrayset_union: every load or store of an element of an isbits-union array was a call, returning the element through a stack slot. OpenCL.jl's copy of the same function has noreturnand is inlined.backend_launch.jland POCL'sargument_types,launch_converted,set_args!andconvert_arguments, which run on every kernel launch.This PR moves
inline_bodyto KernelInterface's utilities and uses it for all of them, plusKI.argument_typesand POCL'sexception_field. A new codegen check verifies that copying an array of isbits unions in a kernel no longer calls either accessor.This was found through JuliaGPU/AcceleratedKernels.jl#209: the non-inlined accessors exposed an LLVM 23 SPIR-V back-end bug, which generated an
OpPhithat failed validation. That bug is fixed separately (llvm/llvm-project#239343, shipped inSPIRV_LLVM_Backend_jll23.1.4+1), so this PR is no longer needed for that.