[SPIR-V] Add -fspv-allow-import for Import linkage in lib targets#1
Open
Jasper-Bekkers wants to merge 4 commits into
Open
[SPIR-V] Add -fspv-allow-import for Import linkage in lib targets#1Jasper-Bekkers wants to merge 4 commits into
Jasper-Bekkers wants to merge 4 commits into
Conversation
Calls to undefined external functions in `lib_*` targets now compile to a body-less OpFunction prototype decorated with `LinkageAttributes ... Import`, so the module can be linked against a separately compiled definition with e.g. spirv-link. The Export side already worked; this is the missing counterpart. Behavior is opt-in via `-fspv-allow-import`. The flag requires a `lib_*` target profile and skips the SPIRV-Tools legalize/optimize stages, since those passes assume every reachable function has a body and would crash on the Import prototypes — optimization is meaningful only after linking. Adds three regression tests covering Import emission, mixed Import+Export in one module, and the non-lib-profile diagnostic. Also adds a test for the existing pure-Export library shape (multiple callables, no entry point). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2 tasks
`-fspv-allow-import` (added in the previous commit) emits body-less `OpFunction` prototypes decorated with `LinkageAttributes Import` for external function references. They're discovered in HLSL source order via `doFunctionDecl`, but in any non-trivial translation unit other function definitions get emitted around them — most pervasively when templates from `#include`d headers are lazily instantiated as the entry point's body references them. SPIR-V Logical Layout (sec. 2.4) requires all function declarations (header-only `OpFunction` ... `OpFunctionEnd`) to precede any function definitions (with body) in the function section. The old single-pass emit produced output like `def, def, decl, def, ...` for these mixed cases, which spirv-val rejects with "Function declarations must appear before function definitions". Add `SpirvFunction::isDeclaration()` (true when `basicBlocks.empty()`) and split the function-emit loop in `SpirvModule::invokeVisitor`'s forward path into two passes: declarations first, definitions second. Insertion order within each pass is preserved, so behaviour is unchanged for translation units that don't carry any Import-linked prototypes. Regression test `lib.fn.import.with.helper.hlsl` covers a translation unit that mixes a body-less Import prototype with a body-having helper function — fails on the old code, passes on the fix.
DXC's emitter normally emits an OpLine before every instruction when `-Zi` is set, including the OpFunctionParameter / OpFunctionEnd instructions inside a body-less Import-linkage prototype. That's structurally valid SPIR-V (the spec allows OpLine almost anywhere in the function section), but it trips spirv-val's layout pass: spirv-val classifies OpLine as belonging to FunctionDefinitions when current section isn't Types (see SPIRV-Tools' `InstructionLayoutSection`), so the moment it sees the first OpLine between an Import prototype's OpFunction and OpFunctionParameter it advances past FunctionDeclarations. The prototype's own OpFunctionEnd is then flagged as a "declaration after definition" violation. Fixing it upstream in SPIRV-Tools would be the right thing — OpLine inside FunctionDeclarations is genuinely valid and the validator's classification is too strict — but that's a slow review cycle and would require redirecting the bundled SPIRV-Tools submodule to a fork. Sidestep it from the DXC side instead: add an `inImportPrototype` flag to EmitVisitor, set on entry to a body-less function and cleared after OpFunctionEnd, and suppress OpLine emit for any instruction inside the prototype's header except OpFunction itself (the OpLine *before* OpFunction sits outside the function and is fine — the Types section accepts OpLine). Cost: Import prototypes lose source-location debug info on their parameters and OpFunctionEnd. Acceptable — the prototype is dropped during link-time import resolution anyway, so the debug info would have been dead. Test: `lib.fn.import.with.helper.hlsl` now compiles with `-Zi` and the FileCheck pattern asserts no OpLine appears between OpFunction and OpFunctionParameter / OpFunctionEnd.
The previous commit suppressed OpLine inside the Import prototype's header (between OpFunction and OpFunctionParameter / OpFunctionEnd), but kept the OpLine that initInstruction emits *immediately before* OpFunction itself. That OpLine sits between the previous emitted function and this prototype. When the previous emit was another Import prototype (i.e. consecutive declarations under the two-pass forward visitor), spirv-val processes that between-prototype OpLine while in FunctionDeclarations layout section. OpLine misclassifies as FunctionDefinitions outside Types, so the validator advances out of FunctionDeclarations — and the *next* prototype's OpFunctionEnd then trips the "declarations must appear before definitions" check. Drop the OpFunction exception. With it, the entire prototype emit window — including the OpLine immediately before OpFunction — has no OpLine. Cost: prototypes lose source-location info on their OpFunction, OpFunctionParameter, and OpFunctionEnd. Acceptable; the prototype is replaced during link-time import resolution and its debug info is dead weight from that point on. Surfaced when shipping the previous commit's binaries to a real user — the local test fixture has only one Import prototype, so the between-prototype OpLine never appeared.
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.
Summary
Three commits:
-fspv-allow-import: new flag (gated tolib_*profiles): calls to undefined external functions compile to a body-lessOpFunctionprototype decorated withLinkageAttributes ... Import, so the resulting SPIR-V module can be linked against a separately compiled definition (e.g. viaspirv-link). Mirrors the existing Export path; the diagnostic atSpirvEmitter.cpp:3235("found undefined function") was the only thing previously blocking this. Skips SPIRV-Tools legalize/optimize stages when the flag is set — those passes assume every reachable function has a body and crash on Import prototypes; legalization is meaningful only after linking.Emit Import prototypes before function definitions: in any non-trivial translation unit, other function definitions get emitted around the prototypes — most pervasively when templates from
#included headers are lazily instantiated as the entry point body references them. SPIR-V Logical Layout (sec. 2.4) requires all function declarations to precede any function definitions, so the naive insertion-order emit produceddef, def, decl, def, ...and spirv-val rejected it with "Function declarations must appear before function definitions". Two-pass forward visit inSpirvModule::invokeVisitor: declarations first, definitions second, insertion order preserved within each pass.Suppress OpLine inside Import prototype headers: with
-Zi, the emitter normally interleaves OpLine before every instruction. Inside an Import prototype that produces an OpLine between OpFunction and the first OpFunctionParameter — structurally valid SPIR-V, but spirv-val classifies OpLine as belonging to FunctionDefinitions outside the Types section, so the layout pass advances past FunctionDeclarations on the first OpLine and the prototype's own OpFunctionEnd is then flagged as a "declaration after definition" violation. The right fix is upstream in SPIRV-Tools (OpLine in FunctionDeclarations is genuinely valid) but that's a slow review cycle that would also require redirecting the bundled SPIRV-Tools submodule to a fork. Sidestep from the DXC side: add aninImportPrototypeflag, set on entry to a body-less function and cleared after OpFunctionEnd, and suppress OpLine emit for any instruction inside the prototype's header except OpFunction itself.End-to-end linking has been verified against the Traverse
spirv-linkercrate: Traverse-Research/spirv-linker#4.Test plan
lib.fn.import.hlsl,lib.fn.import.export.hlsl,spv.allow-import.requires-lib.hlsl,lib.fn.export.callables.hlsl,lib.fn.import.with.helper.hlsl(covers the reorder + OpLine suppression with-Zi).CodeGenSPIRVlit suite: 1566 pass + 2 expected failures, 0 new regressions.export, one calling externals with-fspv-allow-import), link with spirv-linker, assert linked module has noLinkagecapability, noLinkageAttributesdecorations, no body-less functions, and previously-imported names resolve to functions with bodies.🤖 Generated with Claude Code