From 0bc18d233966fc80cf2c482922d0b773d58394ca Mon Sep 17 00:00:00 2001 From: Yong He Date: Mon, 10 Feb 2025 23:48:07 -0800 Subject: Emit missing atomic64 capability, fix int-typed builtin used in both vs and fs. (#6314) Co-authored-by: Ellie Hermaszewska --- source/slang/slang-emit-spirv.cpp | 47 ++++++++++++++++++++++++++++----------- tests/spirv/atomic-64bit.slang | 13 +++++++++++ tests/spirv/view-id.slang | 19 ++++++++++++++++ 3 files changed, 66 insertions(+), 13 deletions(-) create mode 100644 tests/spirv/atomic-64bit.slang create mode 100644 tests/spirv/view-id.slang diff --git a/source/slang/slang-emit-spirv.cpp b/source/slang/slang-emit-spirv.cpp index 4a66cb4ef..ef015df7f 100644 --- a/source/slang/slang-emit-spirv.cpp +++ b/source/slang/slang-emit-spirv.cpp @@ -3192,6 +3192,17 @@ struct SPIRVEmitContext : public SourceEmitterBase, public SPIRVEmitSharedContex void ensureAtomicCapability(IRInst* atomicInst, SpvOp op) { auto typeOp = atomicInst->getDataType()->getOp(); + if (typeOp == kIROp_VoidType) + { + auto ptrType = atomicInst->getOperand(0)->getDataType(); + IRBuilder builder(atomicInst); + if (auto valType = tryGetPointedToType(&builder, ptrType)) + { + if (auto atomicType = as(valType)) + valType = atomicType->getElementType(); + typeOp = valType->getOp(); + } + } switch (op) { case SpvOpAtomicFAddEXT: @@ -5094,18 +5105,23 @@ struct SPIRVEmitContext : public SourceEmitterBase, public SPIRVEmitSharedContex { SpvBuiltIn builtinName; SpvStorageClass storageClass = SpvStorageClassInput; + bool flat = false; BuiltinSpvVarKey() = default; - BuiltinSpvVarKey(SpvBuiltIn builtin, SpvStorageClass storageClass) - : builtinName(builtin), storageClass(storageClass) + BuiltinSpvVarKey(SpvBuiltIn builtin, SpvStorageClass storageClass, bool isFlat) + : builtinName(builtin), storageClass(storageClass), flat(isFlat) { } bool operator==(const BuiltinSpvVarKey& other) const { - return builtinName == other.builtinName && storageClass == other.storageClass; + return builtinName == other.builtinName && storageClass == other.storageClass && + flat == other.flat; } HashCode getHashCode() const { - return combineHash(Slang::getHashCode(builtinName), Slang::getHashCode(storageClass)); + return combineHash( + Slang::getHashCode(builtinName), + Slang::getHashCode(storageClass), + Slang::getHashCode(flat)); } }; Dictionary m_builtinGlobalVars; @@ -5127,26 +5143,25 @@ struct SPIRVEmitContext : public SourceEmitterBase, public SPIRVEmitSharedContex return false; } - void maybeEmitFlatDecorationForBuiltinVar(IRInst* irInst, SpvInst* spvInst) + bool needFlatDecorationForBuiltinVar(IRInst* irInst) { if (!irInst) - return; + return false; if (irInst->getOp() != kIROp_GlobalVar && irInst->getOp() != kIROp_GlobalParam) - return; + return false; auto ptrType = as(irInst->getDataType()); if (!ptrType) - return; + return false; auto addrSpace = ptrType->getAddressSpace(); if (addrSpace == AddressSpace::Input || addrSpace == AddressSpace::BuiltinInput) { if (isIntegralScalarOrCompositeType(ptrType->getValueType())) { if (isInstUsedInStage(irInst, Stage::Fragment)) - _maybeEmitInterpolationModifierDecoration( - IRInterpolationMode::NoInterpolation, - getID(spvInst)); + return true; } } + return false; } SpvInst* getBuiltinGlobalVar(IRType* type, SpvBuiltIn builtinVal, IRInst* irInst) @@ -5155,7 +5170,8 @@ struct SPIRVEmitContext : public SourceEmitterBase, public SPIRVEmitSharedContex auto ptrType = as(type); SLANG_ASSERT(ptrType && "`getBuiltinGlobalVar`: `type` must be ptr type."); auto storageClass = addressSpaceToStorageClass(ptrType->getAddressSpace()); - auto key = BuiltinSpvVarKey(builtinVal, storageClass); + bool isFlat = needFlatDecorationForBuiltinVar(irInst); + auto key = BuiltinSpvVarKey(builtinVal, storageClass, isFlat); if (m_builtinGlobalVars.tryGetValue(key, result)) { return result; @@ -5185,7 +5201,12 @@ struct SPIRVEmitContext : public SourceEmitterBase, public SPIRVEmitSharedContex } m_builtinGlobalVars[key] = varInst; - maybeEmitFlatDecorationForBuiltinVar(irInst, varInst); + if (isFlat) + { + _maybeEmitInterpolationModifierDecoration( + IRInterpolationMode::NoInterpolation, + getID(varInst)); + } return varInst; } diff --git a/tests/spirv/atomic-64bit.slang b/tests/spirv/atomic-64bit.slang new file mode 100644 index 000000000..2b8220dd0 --- /dev/null +++ b/tests/spirv/atomic-64bit.slang @@ -0,0 +1,13 @@ +//TEST:SIMPLE(filecheck=CHECK):-target spirv + +// CHECK: OpCapability Int64Atomics + +groupshared Atomic atomicVariable; + +[shader("compute")] +[numthreads(1, 1, 1)] +void main(uint3 threadId : SV_DispatchThreadID) +{ + uint64_t value = 4ll; + atomicVariable.store(value); +} \ No newline at end of file diff --git a/tests/spirv/view-id.slang b/tests/spirv/view-id.slang new file mode 100644 index 000000000..9df3168a4 --- /dev/null +++ b/tests/spirv/view-id.slang @@ -0,0 +1,19 @@ +//TEST:SIMPLE(filecheck=CHECK): -target spirv + +// ViewIndex builtin should be declared twice, once for vertex and once for fragment shader. +// Because the fragment shader builtin needs Flat, and vertex shader does not. + +// CHECK: OpDecorate %{{.*}} BuiltIn ViewIndex +// CHECK: OpDecorate %{{.*}} BuiltIn ViewIndex + +[shader("vertex")] +float4 vert(int _viewportIndex : SV_ViewID):SV_Position +{ + return float4(_viewportIndex); +} + +[shader("fragment")] +float4 frag(int _viewportIndex : SV_ViewID) : SV_Target +{ + return float4(_viewportIndex, 0, 0, 1); +} \ No newline at end of file -- cgit v1.2.3