Skip to content

SPIRV: hoist constant indexable rvalue temps to Private storage#4237

Open
tycho wants to merge 2 commits into
KhronosGroup:mainfrom
tycho:pr/private-hoist-indexable-temps
Open

SPIRV: hoist constant indexable rvalue temps to Private storage#4237
tycho wants to merge 2 commits into
KhronosGroup:mainfrom
tycho:pr/private-hoist-indexable-temps

Conversation

@tycho

@tycho tycho commented Apr 30, 2026

Copy link
Copy Markdown

Summary

When a const-qualified array (or any rvalue constant composite) is indexed by a runtime expression, glslang materializes the constant into a fresh Function-storage OpVariable at every access site, with an OpStore of the constant immediately preceding the OpAccessChain. This causes a per-invocation memcpy of the entire constant and is catastrophic on real hardware for non-trivial array sizes.

This change emits the temp as a single module-scope Private-storage OpVariable with the constant as its Initializer, memoized per distinct constant. The pattern is well-formed under SPIR-V 1.0 and is a long-recognized form for an immutable lookup table.

Bug

Minimal repro (fragment shader, --target-env opengl, SPIR-V 1.0):

#version 450
layout(location = 0) flat in int index;
layout(location = 0) out float color;
const float table[16] = { 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16 };
void main() { color = table[index]; }

Before this patch:

%indexable = OpVariable %_ptr_Function__arr_float_uint_16 Function
             OpStore %indexable %constArray         ; 64-byte memcpy, every invocation
%ptr       = OpAccessChain %_ptr_Function_float %indexable %index
%val       = OpLoad %float %ptr

For each access site of each constant, glslang emits a fresh Function variable and a fresh OpStore of the entire constant. With multiple access sites of large constants, the per-invocation OpStore traffic scales accordingly.

The bug was originally observed on a real shader, the Jupiter and Io shadertoy by mtnierhoff (https://www.shadertoy.com/view/XXjSRc), running about 480x slower than necessary at 720p on NVIDIA. Buffer A uses two 256-element vec3 constants (about 6 KiB of constant data total). Each is accessed at 4 sites in two helper functions; before this patch, each fragment invocation emitted one Function-storage indexable temp and one OpStore-of-the-full-constant per access site, for a total of about 24 KiB of OpStore traffic per invocation before any sampling work began.

The same shader runs at full speed under D3D11; the DXIL/HLSL toolchain does not emit this pattern. The bug was first reproduced under ANGLE's Vulkan backend, which uses ANGLE's own GLSL-to-SPIR-V translator rather than glslang. ANGLE's translator emitted the same OpStore-of-constant pattern that glslang's pre-1.4 fallback does, and the speedup numbers above come from fixing that pattern in ANGLE. The author of ANGLE's translator (@ShabbyX) suggested checking whether glslang has the analogous bug, which is what produced this patch. The before/after SPIR-V from glslang on this shader and on the affected Buffer A is byte-equivalent in shape to ANGLE's pre/post-fix output.

The behavior of NVIDIA, or any other driver, on the existing SPIR-V 1.4 OpVariable Function ... Initializer + NonWritable form is not measured here. The argument for replacing that form is that the new encoding gives the optimizer strictly more information without relying on any driver-side heuristic, and that it also fixes the original bug for pre-1.4 targets which the existing optimization could not reach.

Why not fix this in spirv-opt

Verified against spirv-opt -O:

  • --scalar-replacement bails because indices are runtime-computed.
  • --eliminate-local-single-store cannot fold OpLoad(OpAccessChain(var, runtime_idx)) to OpCompositeExtract because the latter requires literal indices.
  • --copy-propagate-arrays handles whole-array copies, not this access pattern.

No combination of stock SPIRV-Tools passes recovers the lookup table. The fix needs to live in the front-end.

Prior art

ANGLE landed essentially this fix in its own SPIR-V translator, in two commits earlier today:

This patch mirrors that approach in glslang.

What the patch does

In Builder::accessChainLoad (SPIRV/SpvBuilder.cpp), when the rvalue base of an access chain with a runtime index is an OpConstantComposite, OpConstantCompositeReplicateEXT, or OpConstantNull, emit (or look up a memoized) module-scope OpVariable %ptr Private %const. Use that as the OpAccessChain base. No OpStore needed.

After this patch, the same minimal shader emits:

%28        = OpConstantComposite %_arr_float_uint_16 %float_1 ... %float_16
%indexable = OpVariable %_ptr_Private__arr_float_uint_16 Private %28
       %main = OpFunction ...
         %36 = OpAccessChain %_ptr_Private_float %indexable %32
         %37 = OpLoad %float %36
               OpStore %color %37
               OpReturn
               OpFunctionEnd

The constant is bound to a module-scope Private variable via the Initializer operand. No OpStore of the constant; no per-invocation copy.

Note that the constant value itself was already module-scope before this patch. SPIR-V has no concept of a function-scoped constant; every OpConstant* instruction lives at module scope, regardless of where the GLSL source declared the array. What was function-scoped before this patch was the wrapper OpVariable glslang emitted to give the constant an addressable home for OpAccessChain. This patch moves that wrapper variable to module scope as well.

Memoization across access sites and across functions

N access sites of the same constant share one variable, including across different functions in the shader. New test Test/spv.privateHoist.frag covers this:

const float tableA[8] = { 1.0, 2.0, 3.0, 4.0, 5.0, 6.0, 7.0, 8.0 };
const float tableB[8] = { 8.0, 7.0, 6.0, 5.0, 4.0, 3.0, 2.0, 1.0 };

float helper(int i) {
    return tableA[i & 7] + tableA[(i + 1) & 7];
}

void main() {
    float a0 = tableA[index];
    float a1 = tableA[(index + 1) & 7];
    float b0 = tableB[index];
    float b1 = tableB[(index + 1) & 7];
    float b2 = tableB[(index + 2) & 7];
    float b3 = tableB[(index + 3) & 7];
    float h  = helper(index);
    color = vec4(a0 + a1 + h, b0 + b1 + b2 + b3, 0.0, 1.0);
}

The relevant lines of the emitted SPIR-V (with --target-env opengl):

         %24 = OpConstantComposite %_arr_float_uint_8 %float_1 %float_2 ... %float_8
         %56 = OpConstantComposite %_arr_float_uint_8 %float_8 %float_7 ... %float_1
  %indexable = OpVariable %_ptr_Private__arr_float_uint_8 Private %24
%indexable_0 = OpVariable %_ptr_Private__arr_float_uint_8 Private %56

       %main = OpFunction %void None %3
         %47 = OpAccessChain %_ptr_Private_float %indexable   %46  ; tableA from main
         %53 = OpAccessChain %_ptr_Private_float %indexable   %52  ; tableA from main
         %59 = OpAccessChain %_ptr_Private_float %indexable_0 %57  ; tableB
         %65 = OpAccessChain %_ptr_Private_float %indexable_0 %64  ; tableB
         %72 = OpAccessChain %_ptr_Private_float %indexable_0 %71  ; tableB
         %79 = OpAccessChain %_ptr_Private_float %indexable_0 %78  ; tableB
         %84 = OpFunctionCall %float %helper_i1_ %param
               OpFunctionEnd

 %helper_i1_ = OpFunction %float None %9
         %31 = OpAccessChain %_ptr_Private_float %indexable %27    ; tableA from helper
         %37 = OpAccessChain %_ptr_Private_float %indexable %36    ; tableA from helper
               OpFunctionEnd

Eight OpAccessChain operations across two functions, two module-scope Private variables, zero OpStore instructions targeting either variable. helper() and main() share the same %indexable for tableA. Without memoization this would have been eight distinct Function variables and eight OpStore operations. On the affected shadertoy, memoization collapses the 8 access sites of the two 256-element vec3 arrays from 8 indexable temps to 2.

Entry point interface

For SPIR-V 1.4 and later, statically-used global variables must appear in the entry point interface list. The hoisted vars are merged into iOSet in TGlslangToSpvTraverser (SPIRV/GlslangToSpv.cpp) before OpEntryPoint is finalized. For pre-1.4 targets only Input/Output variables need to appear, so the interface list is unchanged on those targets.

Spec constants

OpSpecConstant* is intentionally excluded. Spec constants can change at pipeline creation time, and hoisting them changes their identity from per-invocation to module-shared.

Comparison with the existing SPIR-V 1.4 Function-with-initializer encoding

Pre-patch, glslang already had a partial optimization at the same call site, gated on SPIR-V 1.4 or later: emit OpVariable Function %const with NonWritable, no OpStore. The new encoding supersedes that one for the only case it actually fired on. Differences worth calling out:

  1. Storage class lifetime. Function-storage variables are local to a function call; their Initializer operand says "set this on entry to the function." Private-storage variables are module-scope with per-invocation lifetime; their Initializer is applied once at the start of the invocation and the variable persists across the function calls that invocation makes. For a helper function called N times, the old encoding can require up to N initializations; the new one can require up to 1.

  2. Memoization. Function variables must be declared in the entry block of their owning function; two functions cannot share one Function variable. The old encoding therefore could not deduplicate the same constant being indexed from multiple functions. Private variables are module-scope and can be shared by every function in the shader, which is the basis for the memoization map this patch adds.

  3. SPIR-V version reach. The old encoding required SPIR-V 1.4 because NonWritable on a Function variable is only legal there. Below 1.4, glslang fell back to the explicit OpStore form, which is the form the original bug arises from. The new encoding works on SPIR-V 1.0 and later, and so fixes the bug on --target-env opengl and --target-env vulkan1.0 / vulkan1.1 (default SPIR-V 1.0 / 1.3) as well.

  4. Decorations. The old encoding decorated the Function variable NonWritable to telegraph "lookup table." The new encoding does not need a decoration: the constant Initializer plus the absence of any OpStore to the variable is itself a strong static signal of immutability, and NonWritable is not a legal decoration on a Private variable in any current SPIR-V version anyway.

The new encoding is at least as good in every dimension the spec lets us reason about, and strictly better on the ones a driver cannot avoid: memoization across access sites is a SPIR-V structural property, not a driver heuristic.

Removing the now-unreachable 1.4+ branch

The previous 1.4+ branch fired when isValidInitializer(accessChain.base) returned true, defined as isConstant(id) || isGlobalVariable(id). After the patch:

  • The constant disjunct is handled by the hoist above the branch.
  • The global-variable disjunct is structurally unreachable. accessChain.base reaches this code via setAccessChainRValue, which has 18 callers in GlslangToSpv.cpp (the unified GLSL+HLSL emitter; HLSL has no separate emitter). Every caller passes a value-typed Id (load result, arithmetic result, constructor result, constant). The dispatch in visitSymbol at GlslangToSpv.cpp:2283 explicitly forks on isPointerType: pointer-typed Ids, which all global variables are, take the lvalue path. No caller passes a global-variable Id to the rvalue path.

The branch is dead from this site. The isValidInitializer predicate itself remains valid in the variable-creation contexts it was originally written for; only this one usage was over-broad.

This cleanup is in a separate commit. If maintainers prefer to land just the hoist for now, the second commit reverts cleanly.

Validation

  • glslang-gtests: 1907 / 1907 pass. 13 existing goldens regenerated for tests that hit the affected code path; all diffs are the mechanical storage-class change (Variable Function <ConstId> becoming Variable Private <ConstId> at module scope) and, for SPIR-V 1.4+ tests, the addition of the new Id to the OpEntryPoint interface list. No semantic differences.
  • Test/runtests (shell suite): "Tests Succeeded."
  • New test spv.privateHoist.frag covers the cross-function memoization invariant.

Tested targets

Env SPV Hoist applied Pvt in iface spirv-val
--target-env opengl 1.0 yes n/a OK (opengl4.5)
--target-env vulkan1.0 1.0 yes n/a OK (vulkan1.0)
--target-env vulkan1.1 1.3 yes n/a OK (vulkan1.1)
--target-env vulkan1.1 --target-env spirv1.4 1.4 yes yes OK (vulkan1.2)
--target-env vulkan1.2 1.5 yes yes OK (vulkan1.2)
--target-env vulkan1.3 1.6 yes yes OK (vulkan1.3)

Note: the vulkan1.1 + spirv1.4 row's module is validated under vulkan1.2 semantics, since SPIR-V 1.4 requires Vulkan 1.2 or VK_KHR_spirv_1_4 to be valid in the host environment.

tycho added 2 commits April 29, 2026 21:47
When a const-qualified array (or any rvalue constant composite) is
indexed by a runtime expression, glslang materialized the constant
into a fresh Function-storage OpVariable at every access site, with
an OpStore of the constant immediately preceding the OpAccessChain.
This caused a per-invocation memcpy of the entire constant and was
catastrophic on real hardware for non-trivial array sizes (~480x
slower at 720p on NVIDIA for one shadertoy with two 256-element vec3
constants accessed at 4 sites each).

This change emits the temp as a single module-scope Private-storage
OpVariable with the constant as its Initializer, memoized per
distinct constant. The pattern is well-formed under SPIR-V 1.0 and
is the canonical form drivers recognize as an immutable lookup
table.

ANGLE landed essentially the same fix in its own SPIR-V translator;
this patch mirrors that approach.

For SPIR-V 1.4+, where statically-used global variables must appear
in the entry point interface list, the hoisted vars are merged into
iOSet before OpEntryPoint is finalized.

Spec constants are intentionally excluded from the hoist: their
value can change at pipeline creation, and hoisting them would
change their identity from per-invocation to module-shared.

Adds Test/spv.privateHoist.frag covering the memoization invariant
(two distinct constants with four access sites each must produce
exactly two module-scope Private variables and zero OpStores of
either constant). Existing goldens regenerated for tests that hit
the affected code path; all diffs are the mechanical storage-class
change and, for SPIR-V 1.4+ tests, the addition of the new Id to
the OpEntryPoint interface list.
The previous 1.4+ branch in Builder::accessChainLoad fired when
isValidInitializer(accessChain.base) returned true, defined as
isConstant(id) || isGlobalVariable(id). After the preceding hoist:

- The constant disjunct is handled by the hoist above the branch.
- The global-variable disjunct is structurally unreachable.
  accessChain.base reaches this code via setAccessChainRValue,
  whose 18 callers in GlslangToSpv.cpp (the unified GLSL+HLSL
  emitter) all pass value-typed Ids. The dispatch in visitSymbol
  explicitly forks on isPointerType: pointer-typed Ids (which all
  global variables are) take the lvalue path. No caller passes a
  global-variable Id to the rvalue path.

The branch is dead from this site. The isValidInitializer helper
itself remains valid in the variable-creation contexts it was
written for; only this one usage was over-broad.

Test suites are unchanged: 1885/1885 gtests and the shell
testsuite both pass without modification.
@CLAassistant

CLAassistant commented Apr 30, 2026

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@dnovillo dnovillo left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks largely OK. Thanks for the detailed description!

Comment thread SPIRV/SpvBuilder.cpp
bool hoistableConstant =
baseOp == Op::OpConstantComposite ||
baseOp == Op::OpConstantCompositeReplicateEXT ||
baseOp == Op::OpConstantNull;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: could you add an explanation on why hoistableConstant is excluding OpSpecConstant*. The PR description states it, but for future changes, it will be useful to have a reminder here on the side-effects of changing the constant at pipeline creation.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, this may read easier if it was factored into a separate helper. Makes the helper testable from Builder too.

@@ -0,0 +1,41 @@
#version 450

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you add a negative test for spec constants here?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants