From 073bf7057fe9d2aed135d770db50c8eaca72c098 Mon Sep 17 00:00:00 2001 From: Jasper Bekkers Date: Sat, 2 May 2026 14:40:57 +0200 Subject: [PATCH 1/4] [SPIR-V] Add -fspv-allow-import for Import linkage in lib targets MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- include/dxc/Support/HLSLOptions.td | 2 + include/dxc/Support/SPIRVOptions.h | 1 + lib/DxcSupport/HLSLOptions.cpp | 3 + tools/clang/lib/SPIRV/DeclResultIdMapper.cpp | 8 +++ tools/clang/lib/SPIRV/SpirvEmitter.cpp | 60 ++++++++++++++++++- tools/clang/lib/SPIRV/SpirvEmitter.h | 11 ++++ .../CodeGenSPIRV/lib.fn.export.callables.hlsl | 38 ++++++++++++ .../CodeGenSPIRV/lib.fn.import.export.hlsl | 18 ++++++ .../test/CodeGenSPIRV/lib.fn.import.hlsl | 29 +++++++++ .../spv.allow-import.requires-lib.hlsl | 6 ++ 10 files changed, 173 insertions(+), 3 deletions(-) create mode 100644 tools/clang/test/CodeGenSPIRV/lib.fn.export.callables.hlsl create mode 100644 tools/clang/test/CodeGenSPIRV/lib.fn.import.export.hlsl create mode 100644 tools/clang/test/CodeGenSPIRV/lib.fn.import.hlsl create mode 100644 tools/clang/test/CodeGenSPIRV/spv.allow-import.requires-lib.hlsl diff --git a/include/dxc/Support/HLSLOptions.td b/include/dxc/Support/HLSLOptions.td index 034214938c..899e0d1309 100644 --- a/include/dxc/Support/HLSLOptions.td +++ b/include/dxc/Support/HLSLOptions.td @@ -397,6 +397,8 @@ def fspv_use_legacy_buffer_matrix_order: Flag<["-"], "fspv-use-legacy-buffer-mat HelpText<"Assume the legacy matrix order (row major) when accessing raw buffers (e.g., ByteAdddressBuffer)">; def fspv_reflect: Flag<["-"], "fspv-reflect">, Group, Flags<[CoreOption, DriverOption]>, HelpText<"Emit additional SPIR-V instructions to aid reflection">; +def fspv_allow_import: Flag<["-"], "fspv-allow-import">, Group, Flags<[CoreOption, DriverOption]>, + HelpText<"Allow undefined external functions in library targets and emit Import linkage decorations for them">; def fspv_debug_EQ : Joined<["-"], "fspv-debug=">, Group, Flags<[CoreOption, DriverOption]>, HelpText<"Specify whitelist of debug info category (file -> source -> line, tool, vulkan-with-source)">; def fspv_extension_EQ : Joined<["-"], "fspv-extension=">, Group, Flags<[CoreOption, DriverOption]>, diff --git a/include/dxc/Support/SPIRVOptions.h b/include/dxc/Support/SPIRVOptions.h index 0253caba63..5472f25903 100644 --- a/include/dxc/Support/SPIRVOptions.h +++ b/include/dxc/Support/SPIRVOptions.h @@ -53,6 +53,7 @@ struct SpirvCodeGenOptions { bool enable16BitTypes = false; bool finiteMathOnly = false; bool enableReflect = false; + bool allowImport = false; bool invertY = false; // Additive inverse bool invertW = false; // Multiplicative inverse bool noWarnEmulatedFeatures = false; diff --git a/lib/DxcSupport/HLSLOptions.cpp b/lib/DxcSupport/HLSLOptions.cpp index 1630243aab..78a4e5c412 100644 --- a/lib/DxcSupport/HLSLOptions.cpp +++ b/lib/DxcSupport/HLSLOptions.cpp @@ -1114,6 +1114,8 @@ int ReadDxcOpts(const OptTable *optionTable, unsigned flagsToInclude, Args.hasFlag(OPT_fspv_use_legacy_buffer_matrix_order, OPT_INVALID, false); opts.SpirvOptions.enableReflect = Args.hasFlag(OPT_fspv_reflect, OPT_INVALID, false); + opts.SpirvOptions.allowImport = + Args.hasFlag(OPT_fspv_allow_import, OPT_INVALID, false); opts.SpirvOptions.noWarnIgnoredFeatures = Args.hasFlag(OPT_Wno_vk_ignored_features, OPT_INVALID, false); opts.SpirvOptions.noWarnEmulatedFeatures = @@ -1296,6 +1298,7 @@ int ReadDxcOpts(const OptTable *optionTable, unsigned flagsToInclude, Args.hasFlag(OPT_fspv_flatten_resource_arrays, OPT_INVALID, false) || Args.hasFlag(OPT_fspv_reduce_load_size, OPT_INVALID, false) || Args.hasFlag(OPT_fspv_reflect, OPT_INVALID, false) || + Args.hasFlag(OPT_fspv_allow_import, OPT_INVALID, false) || Args.hasFlag(OPT_fspv_fix_func_call_arguments, OPT_INVALID, false) || Args.hasFlag(OPT_fspv_print_all, OPT_INVALID, false) || Args.hasFlag(OPT_fspv_use_emulated_heap, OPT_INVALID, false) || diff --git a/tools/clang/lib/SPIRV/DeclResultIdMapper.cpp b/tools/clang/lib/SPIRV/DeclResultIdMapper.cpp index 1a1078bf0b..cff44ca1eb 100644 --- a/tools/clang/lib/SPIRV/DeclResultIdMapper.cpp +++ b/tools/clang/lib/SPIRV/DeclResultIdMapper.cpp @@ -1824,6 +1824,14 @@ SpirvFunction *DeclResultIdMapper::getOrRegisterFn(const FunctionDecl *fn) { if (fn->getAttr()) { spvBuilder.decorateLinkage(nullptr, spirvFunction, fn->getName(), spv::LinkageType::Export, fn->getLocation()); + } else if (spirvOptions.allowImport && !fn->isImplicit()) { + // -fspv-allow-import is gated to lib_* profiles at SpirvEmitter + // construction, so reaching here implies a library compilation. + const FunctionDecl *defn = nullptr; + if (!fn->isDefined(defn)) { + spvBuilder.decorateLinkage(nullptr, spirvFunction, fn->getName(), + spv::LinkageType::Import, fn->getLocation()); + } } // No need to dereference to get the pointer. Function returns that are diff --git a/tools/clang/lib/SPIRV/SpirvEmitter.cpp b/tools/clang/lib/SPIRV/SpirvEmitter.cpp index 288b926f24..98693d0fc1 100644 --- a/tools/clang/lib/SPIRV/SpirvEmitter.cpp +++ b/tools/clang/lib/SPIRV/SpirvEmitter.cpp @@ -613,6 +613,10 @@ SpirvEmitter::SpirvEmitter(CompilerInstance &ci) spvContext.setCurrentShaderModelKind(shaderModel->GetKind()); spvContext.setMajorVersion(shaderModel->GetMajor()); spvContext.setMinorVersion(shaderModel->GetMinor()); + isLibProfile = shaderModel->IsLib(); + + if (spirvOptions.allowImport && !isLibProfile) + emitError("-fspv-allow-import requires a lib_* target profile", {}); spirvOptions.signaturePacking = ci.getCodeGenOpts().HLSLSignaturePackingStrategy == (unsigned)hlsl::DXIL::PackingStrategy::Optimized; @@ -963,8 +967,15 @@ void SpirvEmitter::HandleTranslationUnit(ASTContext &context) { !dsetbindingsToCombineImageSampler.empty() || spirvOptions.signaturePacking; + // SPIR-V modules with Import linkage attributes are not yet linked, so + // SPIRV-Tools' optimizer/legalizer passes (which assume every reachable + // function has a body) are not meaningful and would crash on the + // body-less Import prototypes. The expectation is that the user runs + // those passes after spirv-link has resolved the imports. + const bool skipPostCodegenPasses = spirvOptions.allowImport; + // Run legalization passes - if (spirvOptions.codeGenHighLevel) { + if (spirvOptions.codeGenHighLevel || skipPostCodegenPasses) { beforeHlslLegalization = needsLegalization; } else { if (needsLegalization) { @@ -1561,6 +1572,16 @@ bool SpirvEmitter::handleNodePayloadArrayType(const ParmVarDecl *decl, void SpirvEmitter::doFunctionDecl(const FunctionDecl *decl) { // Forward declaration of a function inside another. if (!decl->isThisDeclarationADefinition()) { + // In library targets with -fspv-allow-import, a function that is + // declared but never defined anywhere in the translation unit is + // emitted as a body-less prototype with Import linkage. The Import + // decoration itself is added by getOrRegisterFn(). + const FunctionDecl *defn = nullptr; + if (isLibProfile && spirvOptions.allowImport && + !decl->isImplicit() && !decl->isDefined(defn)) { + emitImportFunctionPrototype(decl); + return; + } addFunctionToWorkQueue(spvContext.getCurrentShaderModelKind(), decl, /*isEntryFunction*/ false); return; @@ -3232,8 +3253,17 @@ SpirvInstruction *SpirvEmitter::processCall(const CallExpr *callExpr) { // Note that we always want the definition because Stmts/Exprs in the // function body reference the parameters in the definition. if (!callee) { - emitError("found undefined function", callExpr->getExprLoc()); - return nullptr; + // In library targets with -fspv-allow-import, calls to undefined + // functions are emitted as references to an Import-linkage prototype + // so the resulting SPIR-V module can be linked against an external + // definition. + if (isLibProfile && spirvOptions.allowImport) { + callee = callExpr->getDirectCallee(); + } + if (!callee) { + emitError("found undefined function", callExpr->getExprLoc()); + return nullptr; + } } const auto paramTypeMatchesArgType = [](QualType paramType, @@ -15811,6 +15841,30 @@ void SpirvEmitter::addFunctionToWorkQueue(hlsl::DXIL::ShaderKind shaderKind, } } +void SpirvEmitter::emitImportFunctionPrototype(const FunctionDecl *decl) { + // The work queue is idempotent in addFunctionToWorkQueue, so doFunctionDecl + // is only invoked once per external prototype — no further dedup needed. + // getOrRegisterFn applies the Import LinkageAttributes decoration when the + // function has no definition and -fspv-allow-import is set. + SpirvFunction *func = declIdMapper.getOrRegisterFn(decl); + + const QualType retType = + declIdMapper.getTypeAndCreateCounterForPotentialAliasVar(decl); + spvBuilder.beginFunction(retType, decl->getLocStart(), getFnName(decl), + decl->hasAttr(), + decl->hasAttr(), func); + + // OpFunctionParameter for each parameter; no basic blocks (Import + // linkage requires the function body to be absent). + for (uint32_t i = 0; i < decl->getNumParams(); ++i) { + const ParmVarDecl *paramDecl = decl->getParamDecl(i); + declIdMapper.createFnParam(paramDecl, i + 1, + /*decorateIntrinsicAttrs*/ true); + } + + spvBuilder.endFunction(); +} + SpirvInstruction * SpirvEmitter::processTraceRayInline(const CXXMemberCallExpr *expr) { const auto object = expr->getImplicitObjectArgument(); diff --git a/tools/clang/lib/SPIRV/SpirvEmitter.h b/tools/clang/lib/SPIRV/SpirvEmitter.h index 10cc31023c..4c729627cd 100644 --- a/tools/clang/lib/SPIRV/SpirvEmitter.h +++ b/tools/clang/lib/SPIRV/SpirvEmitter.h @@ -1334,6 +1334,12 @@ class SpirvEmitter : public ASTConsumer { const clang::FunctionDecl *, bool isEntryFunction); + /// \brief Emits a body-less SpirvFunction (OpFunction / OpFunctionParameters + /// / OpFunctionEnd, no basic blocks) for the given declaration. Used to + /// model external functions for SPIR-V Import linkage when the user passes + /// -fspv-allow-import on a library target. + void emitImportFunctionPrototype(const clang::FunctionDecl *decl); + /// \brief Helper function to run SPIRV-Tools optimizer's performance passes. /// Runs the SPIRV-Tools optimizer on the given SPIR-V module |mod|, and /// gets the info/warning/error messages via |messages|. @@ -1583,6 +1589,11 @@ class SpirvEmitter : public ASTConsumer { /// option to spirv-val because of illegal function parameter scope. bool beforeHlslLegalization; + /// True when the overall compilation profile is `lib_*`. Stays true even + /// while emitting an entry function within the library, where + /// `spvContext.isLib()` flips to the entry's per-stage shader model kind. + bool isLibProfile = false; + /// Mapping from methods to the decls to represent their implicit object /// parameters /// diff --git a/tools/clang/test/CodeGenSPIRV/lib.fn.export.callables.hlsl b/tools/clang/test/CodeGenSPIRV/lib.fn.export.callables.hlsl new file mode 100644 index 0000000000..d843ca83c3 --- /dev/null +++ b/tools/clang/test/CodeGenSPIRV/lib.fn.export.callables.hlsl @@ -0,0 +1,38 @@ +// RUN: %dxc -T lib_6_x -fspv-target-env=universal1.5 %s -spirv | FileCheck %s + +// Library with multiple exported callable functions and no entry point: a +// SPIR-V module suitable for spirv-link consumption. Mirrors the +// material-graph "node" shape: every callable takes a struct as its first +// in-parameter, returns through an `out` parameter, and ranges over scalar +// and vector floats plus a few common intrinsics. + +// CHECK-DAG: OpCapability Shader +// CHECK-DAG: OpCapability Linkage + +// No entry points should be emitted for an export-only library. +// CHECK-NOT: OpEntryPoint + +// CHECK-DAG: OpDecorate %add_f1 LinkageAttributes "add_f1" Export +// CHECK-DAG: OpDecorate %add_f4 LinkageAttributes "add_f4" Export +// CHECK-DAG: OpDecorate %dot_f3 LinkageAttributes "dot_f3" Export +// CHECK-DAG: OpDecorate %normalize_f3 LinkageAttributes "normalize_f3" Export +// CHECK-DAG: OpDecorate %saturate_f1 LinkageAttributes "saturate_f1" Export +// CHECK-DAG: OpDecorate %lerp_f4 LinkageAttributes "lerp_f4" Export + +struct ShadingContext { + uint3 dispatch_thread_id; + uint num_invocations; +}; + +export void add_f1(in ShadingContext ctx, in float a, in float b, out float result) { result = a + b; } +export void add_f4(in ShadingContext ctx, in float4 a, in float4 b, out float4 result) { result = a + b; } + +export void dot_f3(in ShadingContext ctx, in float3 a, in float3 b, out float result) { result = dot(a, b); } + +export void normalize_f3(in ShadingContext ctx, in float3 a, out float3 result) { result = normalize(a); } + +export void saturate_f1(in ShadingContext ctx, in float a, out float result) { result = saturate(a); } + +export void lerp_f4(in ShadingContext ctx, in float4 a, in float4 b, in float t, out float4 result) { + result = lerp(a, b, t); +} diff --git a/tools/clang/test/CodeGenSPIRV/lib.fn.import.export.hlsl b/tools/clang/test/CodeGenSPIRV/lib.fn.import.export.hlsl new file mode 100644 index 0000000000..f09bb5bf45 --- /dev/null +++ b/tools/clang/test/CodeGenSPIRV/lib.fn.import.export.hlsl @@ -0,0 +1,18 @@ +// RUN: %dxc -T lib_6_x -fspv-target-env=universal1.5 -fspv-allow-import %s -spirv | FileCheck %s + +// Library target with both Export and Import linkage in the same module: +// `caller` is exported and forwards through an undefined `helper` which +// must be Import-linked. + +// CHECK-DAG: OpCapability Linkage +// CHECK-DAG: OpDecorate %caller LinkageAttributes "caller" Export +// CHECK-DAG: OpDecorate %helper LinkageAttributes "helper" Import + +// No entry points should be emitted. +// CHECK-NOT: OpEntryPoint + +float helper(float x); + +export float caller(float v) { + return helper(v); +} diff --git a/tools/clang/test/CodeGenSPIRV/lib.fn.import.hlsl b/tools/clang/test/CodeGenSPIRV/lib.fn.import.hlsl new file mode 100644 index 0000000000..97d682683d --- /dev/null +++ b/tools/clang/test/CodeGenSPIRV/lib.fn.import.hlsl @@ -0,0 +1,29 @@ +// RUN: %dxc -T lib_6_x -E main -fspv-target-env=universal1.5 -fspv-allow-import %s -spirv | FileCheck %s + +// Calls to undefined external functions in a library target compile to a +// body-less OpFunction prototype decorated with Import linkage, so the +// resulting SPIR-V module can be linked against a separately compiled +// definition. + +// CHECK-DAG: OpCapability Linkage + +// The Import prototype must be referenced as the callee of OpFunctionCall. +// CHECK-DAG: OpDecorate %external_helper LinkageAttributes "external_helper" Import + +// CHECK: OpFunctionCall %void %external_helper + +// The prototype itself must have parameters but no entry block. +// CHECK: %external_helper = OpFunction %void None +// CHECK-NEXT: OpFunctionParameter +// CHECK-NEXT: OpFunctionParameter +// CHECK-NEXT: OpFunctionEnd + +void external_helper(float a, out float result); + +[shader("compute")] +[numthreads(1, 1, 1)] +void main(uint3 dtid : SV_DispatchThreadID) +{ + float result; + external_helper(1.0, result); +} diff --git a/tools/clang/test/CodeGenSPIRV/spv.allow-import.requires-lib.hlsl b/tools/clang/test/CodeGenSPIRV/spv.allow-import.requires-lib.hlsl new file mode 100644 index 0000000000..4034f42acd --- /dev/null +++ b/tools/clang/test/CodeGenSPIRV/spv.allow-import.requires-lib.hlsl @@ -0,0 +1,6 @@ +// RUN: not %dxc -T cs_6_0 -E main -fspv-target-env=universal1.5 -fspv-allow-import %s -spirv 2>&1 | FileCheck %s + +// CHECK: error: -fspv-allow-import requires a lib_* target profile + +[numthreads(1,1,1)] +void main() {} From a153d3278b308a28b8e69aed1bd32e2df690fb21 Mon Sep 17 00:00:00 2001 From: Jasper Bekkers Date: Sat, 2 May 2026 23:14:30 +0200 Subject: [PATCH 2/4] [SPIR-V] Emit Import prototypes before function definitions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `-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. --- .../clang/include/clang/SPIRV/SpirvFunction.h | 6 ++++ tools/clang/lib/SPIRV/SpirvModule.cpp | 18 ++++++++-- .../lib.fn.import.with.helper.hlsl | 33 +++++++++++++++++++ 3 files changed, 55 insertions(+), 2 deletions(-) create mode 100644 tools/clang/test/CodeGenSPIRV/lib.fn.import.with.helper.hlsl diff --git a/tools/clang/include/clang/SPIRV/SpirvFunction.h b/tools/clang/include/clang/SPIRV/SpirvFunction.h index 88542255f4..0752865f97 100644 --- a/tools/clang/include/clang/SPIRV/SpirvFunction.h +++ b/tools/clang/include/clang/SPIRV/SpirvFunction.h @@ -94,6 +94,12 @@ class SpirvFunction { void addVariable(SpirvVariable *); void addBasicBlock(SpirvBasicBlock *); + // True if this is a function declaration (header-only, no body), as + // emitted under -fspv-allow-import for undefined external functions. + // Per SPIR-V Logical Layout (sec. 2.4), declarations precede + // definitions in the function section. + bool isDeclaration() const { return basicBlocks.empty(); } + /// Adds the given instruction as the first instruction of this SPIR-V /// function body. void addFirstInstruction(SpirvInstruction *inst) { diff --git a/tools/clang/lib/SPIRV/SpirvModule.cpp b/tools/clang/lib/SPIRV/SpirvModule.cpp index 4ba707548f..0fec6375c3 100644 --- a/tools/clang/lib/SPIRV/SpirvModule.cpp +++ b/tools/clang/lib/SPIRV/SpirvModule.cpp @@ -223,9 +223,23 @@ bool SpirvModule::invokeVisitor(Visitor *visitor, bool reverseOrder) { if (!debugInstructions[i]->invokeVisitor(visitor)) return false; + // SPIR-V Logical Layout (sec. 2.4) requires all function + // declarations (header-only) to precede any function definitions + // (with bodies). Under -fspv-allow-import we emit body-less + // prototypes for undefined external functions, but they're + // discovered in source order — interleaved with bodies coming + // from the same translation unit (e.g. helpers pulled in via + // #include). Two-pass emit keeps declarations first, definitions + // second; spirv-val rejects the binary otherwise with "Function + // declarations must appear before function definitions". for (auto fn : functions) - if (!fn->invokeVisitor(visitor, reverseOrder)) - return false; + if (fn->isDeclaration()) + if (!fn->invokeVisitor(visitor, reverseOrder)) + return false; + for (auto fn : functions) + if (!fn->isDeclaration()) + if (!fn->invokeVisitor(visitor, reverseOrder)) + return false; } if (!visitor->visit(this, Visitor::Phase::Done)) diff --git a/tools/clang/test/CodeGenSPIRV/lib.fn.import.with.helper.hlsl b/tools/clang/test/CodeGenSPIRV/lib.fn.import.with.helper.hlsl new file mode 100644 index 0000000000..8e0175fedf --- /dev/null +++ b/tools/clang/test/CodeGenSPIRV/lib.fn.import.with.helper.hlsl @@ -0,0 +1,33 @@ +// RUN: %dxc -T lib_6_x -fspv-target-env=universal1.5 -fspv-allow-import %s -spirv | FileCheck %s + +// Regression test for SPIR-V Logical Layout (sec. 2.4): when a +// translation unit mixes a body-less Import prototype with one or more +// helper functions that *do* have bodies, the prototype must come +// first in the function section. A naive insertion-order emit produces +// `helper_def -> import_decl -> caller_def` which spirv-val rejects +// with "Function declarations must appear before function definitions". +// +// This case mirrors what the breda material-graph stub looks like: a +// body-less external (resolved at link time) plus a body-having +// helper function from an inlined header, both called from the +// exported `caller`. + +// CHECK-DAG: OpCapability Linkage +// CHECK-DAG: OpDecorate %external_helper LinkageAttributes "external_helper" Import + +// The Import prototype must appear before any function definitions. +// CHECK: %external_helper = OpFunction %void None +// CHECK-NEXT: OpFunctionParameter +// CHECK-NEXT: OpFunctionEnd +// CHECK: %inline_helper = OpFunction +// CHECK: %caller = OpFunction + +float external_helper(float a); + +float inline_helper(float a) { + return a * 2.0; +} + +export float caller(float v) { + return external_helper(v) + inline_helper(v); +} From eafa33f71f21010e77cee284e24381ca19f6cad2 Mon Sep 17 00:00:00 2001 From: Jasper Bekkers Date: Sat, 2 May 2026 23:20:38 +0200 Subject: [PATCH 3/4] [SPIR-V] Suppress OpLine inside Import prototypes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- tools/clang/lib/SPIRV/EmitVisitor.cpp | 22 +++++++++++ tools/clang/lib/SPIRV/EmitVisitor.h | 13 ++++++- .../lib.fn.import.with.helper.hlsl | 38 +++++++++++++------ 3 files changed, 60 insertions(+), 13 deletions(-) diff --git a/tools/clang/lib/SPIRV/EmitVisitor.cpp b/tools/clang/lib/SPIRV/EmitVisitor.cpp index fcdcf5bcd0..441b82a010 100644 --- a/tools/clang/lib/SPIRV/EmitVisitor.cpp +++ b/tools/clang/lib/SPIRV/EmitVisitor.cpp @@ -280,6 +280,21 @@ void EmitVisitor::emitDebugLine(spv::Op op, const SourceLocation &loc, (op != spv::Op::OpReturn && op != spv::Op::OpFunction)) return; + // Body-less Import prototype: suppress OpLine for any instruction + // inside the prototype's header (OpFunctionParameter, OpFunctionEnd). + // OpFunction itself is excluded — the OpLine that precedes it sits + // outside the function and is processed in the Types layout section, + // which is fine. The OpLines that the validator chokes on are the + // ones between OpFunction and OpFunctionParameter and between + // parameters / OpFunctionEnd; under spirv-val's current layout + // classification (OpLine -> FunctionDefinitions outside Types) those + // advance the section past FunctionDeclarations and trip the + // "declaration after definition" check on the prototype's own + // OpFunctionEnd. Skipping them here keeps the binary + // structurally-valid AND validator-clean. + if (inImportPrototype && op != spv::Op::OpFunction) + return; + // Based on SPIR-V spec, OpSelectionMerge must immediately precede either an // OpBranchConditional or OpSwitch instruction. Similarly OpLoopMerge must // immediately precede either an OpBranch or OpBranchConditional instruction. @@ -522,6 +537,12 @@ bool EmitVisitor::visit(SpirvFunction *fn, Phase phase) { if (fn->isEntryFunctionWrapper()) inEntryFunctionWrapper = true; + // Body-less functions are Import prototypes (under + // -fspv-allow-import). Suppress OpLine for instructions inside + // their header — see the `inImportPrototype` flag in EmitVisitor.h + // for the rationale. + if (fn->isDeclaration()) + inImportPrototype = true; // Emit OpFunction initInstruction(spv::Op::OpFunction, fn->getSourceLocation()); @@ -547,6 +568,7 @@ bool EmitVisitor::visit(SpirvFunction *fn, Phase phase) { initInstruction(spv::Op::OpFunctionEnd, /* SourceLocation */ {}); finalizeInstruction(&mainBinary); inEntryFunctionWrapper = false; + inImportPrototype = false; } return true; diff --git a/tools/clang/lib/SPIRV/EmitVisitor.h b/tools/clang/lib/SPIRV/EmitVisitor.h index d123243448..8dda1337e0 100644 --- a/tools/clang/lib/SPIRV/EmitVisitor.h +++ b/tools/clang/lib/SPIRV/EmitVisitor.h @@ -221,7 +221,7 @@ class EmitVisitor : public Visitor { debugMainFileId(0), debugInfoExtInstId(0), debugLineStart(0), debugLineEnd(0), debugColumnStart(0), debugColumnEnd(0), lastOpWasMergeInst(false), inEntryFunctionWrapper(false), - hlslVersion(0) {} + inImportPrototype(false), hlslVersion(0) {} ~EmitVisitor(); @@ -493,6 +493,17 @@ class EmitVisitor : public Visitor { bool lastOpWasMergeInst; // True if currently it enters an entry function wrapper. bool inEntryFunctionWrapper; + // True while emitting a body-less Import-linkage function prototype + // (the form -fspv-allow-import emits for undefined external functions). + // Used by emitDebugLine to suppress OpLine instructions inside the + // prototype's header — they're structurally legal per SPIR-V spec but + // confuse spirv-val's layout pass into advancing past + // FunctionDeclarations on the first OpLine, which then flags the + // prototype's own OpFunctionEnd as a "declaration after definition" + // violation. See `external/SPIRV-Tools` upstream issue: OpLine in + // FunctionDeclarations layout section is misclassified as + // FunctionDefinitions. + bool inImportPrototype; // Map of filename string id to the id of its DebugSource instruction. When // generating OpSource instruction without a result id, use 1 to remember it // was generated. diff --git a/tools/clang/test/CodeGenSPIRV/lib.fn.import.with.helper.hlsl b/tools/clang/test/CodeGenSPIRV/lib.fn.import.with.helper.hlsl index 8e0175fedf..994e9d4b46 100644 --- a/tools/clang/test/CodeGenSPIRV/lib.fn.import.with.helper.hlsl +++ b/tools/clang/test/CodeGenSPIRV/lib.fn.import.with.helper.hlsl @@ -1,21 +1,35 @@ -// RUN: %dxc -T lib_6_x -fspv-target-env=universal1.5 -fspv-allow-import %s -spirv | FileCheck %s +// RUN: %dxc -T lib_6_x -fspv-target-env=universal1.5 -fspv-allow-import -Zi %s -spirv | FileCheck %s -// Regression test for SPIR-V Logical Layout (sec. 2.4): when a -// translation unit mixes a body-less Import prototype with one or more -// helper functions that *do* have bodies, the prototype must come -// first in the function section. A naive insertion-order emit produces -// `helper_def -> import_decl -> caller_def` which spirv-val rejects -// with "Function declarations must appear before function definitions". +// Regression test for two SPIR-V Logical Layout issues that surface +// when a translation unit mixes a body-less Import prototype with one +// or more helper functions that have bodies (which is what the breda +// material-graph stub looks like: a body-less external resolved at +// link time, plus body-having helpers from an inlined header): // -// This case mirrors what the breda material-graph stub looks like: a -// body-less external (resolved at link time) plus a body-having -// helper function from an inlined header, both called from the -// exported `caller`. +// 1. The prototype must come first in the function section. A naive +// insertion-order emit produces `helper_def -> import_decl -> +// caller_def` which spirv-val rejects with "Function declarations +// must appear before function definitions". +// +// 2. With `-Zi`, OpLine instructions normally interleave between +// OpFunction / OpFunctionParameter / OpFunctionEnd. Inside an +// Import prototype the OpLine between OpFunction and the first +// OpFunctionParameter trips spirv-val's layout pass into advancing +// past `FunctionDeclarations` (it classifies OpLine as +// FunctionDefinitions outside the Types section), and the +// prototype's own OpFunctionEnd is then flagged as a "declaration +// after definition" violation. +// +// EmitVisitor's `inImportPrototype` flag suppresses OpLine for every +// instruction inside a body-less prototype's header except OpFunction +// itself. The OpLine before OpFunction is fine — it's processed in +// the Types section. // CHECK-DAG: OpCapability Linkage // CHECK-DAG: OpDecorate %external_helper LinkageAttributes "external_helper" Import -// The Import prototype must appear before any function definitions. +// The Import prototype must appear before any function definitions +// AND must contain no OpLine instructions inside its header. // CHECK: %external_helper = OpFunction %void None // CHECK-NEXT: OpFunctionParameter // CHECK-NEXT: OpFunctionEnd From c99070ed9164fb96b2afa0765294a3396adf36eb Mon Sep 17 00:00:00 2001 From: Jasper Bekkers Date: Sun, 3 May 2026 13:53:26 +0200 Subject: [PATCH 4/4] [SPIR-V] Suppress OpLine before Import prototypes too MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- tools/clang/lib/SPIRV/EmitVisitor.cpp | 28 ++++++++++++++------------- 1 file changed, 15 insertions(+), 13 deletions(-) diff --git a/tools/clang/lib/SPIRV/EmitVisitor.cpp b/tools/clang/lib/SPIRV/EmitVisitor.cpp index 441b82a010..7ea6fa8a4b 100644 --- a/tools/clang/lib/SPIRV/EmitVisitor.cpp +++ b/tools/clang/lib/SPIRV/EmitVisitor.cpp @@ -280,19 +280,21 @@ void EmitVisitor::emitDebugLine(spv::Op op, const SourceLocation &loc, (op != spv::Op::OpReturn && op != spv::Op::OpFunction)) return; - // Body-less Import prototype: suppress OpLine for any instruction - // inside the prototype's header (OpFunctionParameter, OpFunctionEnd). - // OpFunction itself is excluded — the OpLine that precedes it sits - // outside the function and is processed in the Types layout section, - // which is fine. The OpLines that the validator chokes on are the - // ones between OpFunction and OpFunctionParameter and between - // parameters / OpFunctionEnd; under spirv-val's current layout - // classification (OpLine -> FunctionDefinitions outside Types) those - // advance the section past FunctionDeclarations and trip the - // "declaration after definition" check on the prototype's own - // OpFunctionEnd. Skipping them here keeps the binary - // structurally-valid AND validator-clean. - if (inImportPrototype && op != spv::Op::OpFunction) + // Body-less Import prototype: suppress OpLine for the *entire* emit + // window — including the OpLine that initInstruction would emit + // immediately before OpFunction itself. spirv-val's current layout + // classification (OpLine -> FunctionDefinitions outside Types) + // advances the section past FunctionDeclarations on any OpLine seen + // while in FunctionDeclarations: the OpLines between OpFunction and + // OpFunctionParameter trip the prototype's *own* OpFunctionEnd, and + // — more subtly — the OpLine that DXC emits *between* two + // consecutive prototypes (after the previous OpFunctionEnd, before + // this prototype's OpFunction) trips the *next* prototype's + // OpFunctionEnd. Skipping every OpLine inside the prototype's emit + // window keeps the binary structurally-valid AND validator-clean. + // Prototypes are dropped during link-time import resolution anyway, + // so the lost source-location info on them costs nothing. + if (inImportPrototype) return; // Based on SPIR-V spec, OpSelectionMerge must immediately precede either an