codegen: store constant union members inline instead of boxing#62191
codegen: store constant union members inline instead of boxing#62191maleadt wants to merge 2 commits into
Conversation
|
Maybe a codegen test? |
|
Added a test. |
0927608 to
02b31ff
Compare
|
This is a de-optimization for normal targets, since it means that a Do we need a codegen param to disable carrying those opportunistic boxes around on GPU targets? |
|
Agreed,though furthermore this feels conceptually illogical and integrated in the wrong place |
…mbers Since #55045 a small isbits Union built from a constant (e.g. `x = cond ? a::Int32 : Int64(0)`) is represented fully boxed: the payload slot is never written and consumers read the value through an embedded pointer to the boxed constant. This breaks back-ends that cannot dereference host addresses (e.g. GPU). Add an `embed_pointers` CodegenParam (default true); when disabled, store a constant whose type is an unboxed leaf of the target union inline with an unmarked tindex. The default retains the boxed representation, so CPU codegen is unaffected. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
02b31ff to
3ae7db5
Compare
|
Okay. Implemented as a cgparam instead. |
|
This isn't illogical because GPUs operate under different logic (they can't access our globals). But a cgparam seems like a good 2nd option |
|
Is it possible to apply this as a transform pass instead? Having to maintain all of the places we eagerly emit a boxed pointer is not going to age very well - We expect to do this more in future, and I don't think this covers all of the cases that we do it already |
And how would that work? |
|
It's certainly awkward. We can lift a The alternative is branching everywhere that we eagerly try to forward boxes from |
|
Maybe I'm misunderstanding, but I think I'd rather simply emit the isbits value rather than have to deal with copying CPU boxes to the GPU and rewrite the IR to make them addressable? We already have to deal with this in GPUCompiler for e.g. type tags, and having a cgparam could be useful to tie other emission patterns to. I pushed a commit that prototypes your |
Route constant values through a centralized maybe_boxed helper before split-union construction can opportunistically forward boxed constants. This preserves the host default while letting pointer-free constants materialize as inline bits when embedded object pointers are disabled. Co-authored-by: OpenAI Codex <codex@openai.com> Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
cb73785 to
8a33c0f
Compare
| If enabled, generated code may embed pointers to host-resident boxed Julia | ||
| objects. Disable this for targets that cannot dereference such pointers | ||
| when codegen can instead use an unboxed representation. | ||
| """ | ||
| embed_pointers::Cint |
There was a problem hiding this comment.
Wording wise, it does mention boxed objects, but embed_pointers implies something about singleton values like Symbol and Type which we do want to support on the GPU.
There was a problem hiding this comment.
True, but I was hoping we could also avoid some of that pointer embedding over time, hence keeping the language more neutral. Although to be honest I haven't really considered how we'd do that.
|
Trying something out in GPUCompiler.jl instead: JuliaGPU/GPUCompiler.jl#874 |
|
Done in GPUCompiler.jl instead: JuliaGPU/GPUCompiler.jl#874 |
Since #55045 a small isbits
Unionbuilt from a constant (e.g.x = cond ? a::Int32 : 0) is represented fully boxed: the unboxed payload slot is never written and consumers read the value back through aliteral_pointer_valof the boxed constant.mark_julia_constmarks constants asisboxed, so the union-phi path keeps them boxed rather than storing them inline as it did before. The embedded object pointer is dead weight and breaks consumers that can't dereference host addresses (e.g. GPU back-ends). Detect a constant whose type is an unboxed leaf of the target union and store its bits inline with an unmarked tindex, restoring the pre-#55045 representation.