diff options
| author | ArielG-NV <159081215+ArielG-NV@users.noreply.github.com> | 2024-06-13 19:12:42 -0400 |
|---|---|---|
| committer | GitHub <noreply@github.com> | 2024-06-13 16:12:42 -0700 |
| commit | 2407966e899f9e4f490b23a92fc06d5da20544cc (patch) | |
| tree | 42472b1420a61441cb9d70a8ba860310b0531e13 | |
| parent | fba316f0e7dacc7f93bee3a95fb93b2ab02bdd80 (diff) | |
SPIR-V `SV_InstanceId` support in pixel shader (#4368)
* solve issue by making 'non SV for SPIRV' system semantics, no longer an SV once diagnosed by slang
Problem:
1. SV_InstanceID (HLSL) is allowed as an output semantic with HLSL, it is also allowed for input into pixel shader. We need to account for multiple entry points in 1 file using both of these scenarios at once.
2. SPIRV does not allow `SV_InstanceID` as a builtin, `SV_InstanceID` must be passed as a regular `flat` `Input` from vertex shader.
Solution: Slang needs to treat these SV objects as non built-in's. As a result:
1. Slang needs to allocate for vertex output and fragment Input binding slots for all SV_InstanceID uses (if the target is SPIRV). This allows Slang to prepare an open slot to bind these SV objects to.
2. Slang needs to not emit built in modifier for these not actual built in variables (under the specific circumstances described).
note: `VarLayout` was made not `HOISTABLE` since I don't believe it needs to be `HOISTABLE`.
* The code can be made to work even if `VarLayout` is `HOISTABLE`.
* fix compile warnings
* address review comments and reimplement operand removal
1. remove an operand by selectively copying operands
2. test to ensure `Flat` is in the test shaders
* we do not need to manually add `Flat` in the code since this is done for us in emit-spirv. This is the behavior since `uint` on a fragment `Input` must be `Flat`, else it is a VK validation error.
* address review
remove clone logic
* address review
remove unused function
reserve instead of setCount with ShortList
| -rw-r--r-- | source/slang/slang-ir-glsl-legalize.cpp | 34 | ||||
| -rw-r--r-- | source/slang/slang-ir-util.h | 7 | ||||
| -rw-r--r-- | source/slang/slang-parameter-binding.cpp | 15 | ||||
| -rw-r--r-- | tests/bugs/gh-3087-multi-entry-point.slang | 38 | ||||
| -rw-r--r-- | tests/bugs/gh-3087.slang | 28 |
5 files changed, 119 insertions, 3 deletions
diff --git a/source/slang/slang-ir-glsl-legalize.cpp b/source/slang/slang-ir-glsl-legalize.cpp index ae30184c8..be222c87f 100644 --- a/source/slang/slang-ir-glsl-legalize.cpp +++ b/source/slang/slang-ir-glsl-legalize.cpp @@ -1285,6 +1285,36 @@ ScalarizedVal createSimpleGLSLGlobalVarying( declarator, &systemValueInfoStorage); + { + + auto systemSemantic = inVarLayout->findAttr<IRSystemValueSemanticAttr>(); + // Validate the system value, convert to a regular parameter if this is not a valid system value for a given target. + if (systemSemantic && isSPIRV(codeGenContext->getTargetFormat()) && systemSemantic->getName().caseInsensitiveEquals(UnownedStringSlice("sv_instanceid")) + && ((stage == Stage::Fragment) || (stage == Stage::Vertex && inVarLayout->usesResourceKind(LayoutResourceKind::VaryingOutput)))) + { + ShortList<IRInst*> newOperands; + auto opCount = inVarLayout->getOperandCount(); + newOperands.reserveOverflowBuffer(opCount); + for (UInt i = 0; i < opCount; ++i) + { + auto op = inVarLayout->getOperand(i); + if (op == systemSemantic) + continue; + newOperands.add(op); + } + + auto newVarLayout = builder->emitIntrinsicInst( + inVarLayout->getFullType(), + inVarLayout->getOp(), + newOperands.getCount(), + newOperands.getArrayView().getBuffer()); + + newVarLayout->sourceLoc = inVarLayout->sourceLoc; + + inVarLayout->replaceUsesWith(newVarLayout); + } + } + IRType* type = inType; IRType* peeledRequiredType = nullptr; @@ -2570,9 +2600,7 @@ static void legalizeMeshOutputParam( // // First, collect the subset of outputs being used - const bool isSPIRV = codeGenContext->getTargetFormat() == CodeGenTarget::SPIRV - || codeGenContext->getTargetFormat() == CodeGenTarget::SPIRVAssembly; - if(!isSPIRV) + if(!isSPIRV(codeGenContext->getTargetFormat())) { auto isMeshOutputBuiltin = [](IRInst* g) { diff --git a/source/slang/slang-ir-util.h b/source/slang/slang-ir-util.h index 03a45746d..d78ceaaf8 100644 --- a/source/slang/slang-ir-util.h +++ b/source/slang/slang-ir-util.h @@ -338,6 +338,13 @@ void verifyComputeDerivativeGroupModifiers( bool linearAttr, IRNumThreadsDecoration* numThreadsDecor); + +inline bool isSPIRV(CodeGenTarget codeGenTarget) +{ + return codeGenTarget == CodeGenTarget::SPIRV + || codeGenTarget == CodeGenTarget::SPIRVAssembly; +} + } #endif diff --git a/source/slang/slang-parameter-binding.cpp b/source/slang/slang-parameter-binding.cpp index bce9b5d05..ca0de9802 100644 --- a/source/slang/slang-parameter-binding.cpp +++ b/source/slang/slang-parameter-binding.cpp @@ -4,6 +4,7 @@ #include "slang-lookup.h" #include "slang-compiler.h" #include "slang-type-layout.h" +#include "slang-ir-util.h" #include "../compiler-core/slang-artifact-desc-util.h" @@ -1640,6 +1641,20 @@ static RefPtr<TypeLayout> processSimpleEntryPointParameter( type, kEntryPointParameterDirection_Output); } + else if (isSPIRV(context->getTargetRequest()->getTarget()) + && ( + (state.directionMask & kEntryPointParameterDirection_Input && state.stage == Stage::Fragment) + || (state.directionMask & kEntryPointParameterDirection_Output && state.stage == Stage::Vertex) + ) + && sn == "sv_instanceid" + ) + { + // This fragment-shader-input/vertex-shader-output is effectively not a system semantic for SPIR-V, + typeLayout = getSimpleVaryingParameterTypeLayout( + context->layoutContext, + type, + state.directionMask); + } else { // For a system-value parameter (that didn't match the diff --git a/tests/bugs/gh-3087-multi-entry-point.slang b/tests/bugs/gh-3087-multi-entry-point.slang new file mode 100644 index 000000000..1bbbb9995 --- /dev/null +++ b/tests/bugs/gh-3087-multi-entry-point.slang @@ -0,0 +1,38 @@ +//TEST:SIMPLE(filecheck=CHECK): -target spirv -fvk-use-entrypoint-name -emit-spirv-directly + +// CHECK-DAG: OpEntryPoint Vertex +// CHECK-DAG: OpEntryPoint Fragment + +// we should only have 1 'BuiltIn InstanceIndex' since the `Output` and `Input` semantic +// for `InstanceIndex` should be converted to a non-builtin +// CHECK-DAG: BuiltIn InstanceIndex +// CHECK-NOT: BuiltIn InstanceIndex + +// We require 1 `Flat` for the fragment input `uint` +// SPIRV: Flat +// SPIRV-NOT: Flat + +struct VIn +{ + uint instanceID : SV_InstanceID; +} + +struct VSOutput +{ + uint instanceID : SV_InstanceID; + float4 color : COLOR; +}; + +[shader("vertex")] +VSOutput vmain(VIn vin) { + VSOutput t; + t.instanceID = vin.instanceID; + t.color = float4(0, 0, 0, 0); + return t; +} + +[shader("pixel")] +float4 pmain(VSOutput input) : SV_TARGET +{ + return float4(float(input.instanceID), input.color.xyz); +} diff --git a/tests/bugs/gh-3087.slang b/tests/bugs/gh-3087.slang new file mode 100644 index 000000000..1f6fa98ce --- /dev/null +++ b/tests/bugs/gh-3087.slang @@ -0,0 +1,28 @@ +//TEST:SIMPLE(filecheck=HLSL): -entry main -target hlsl +//TEST:SIMPLE(filecheck=GLSL): -entry main -target glsl +//TEST:SIMPLE(filecheck=SPIRV): -entry main -target spirv + +// HLSL-DAG: main +// HLSL-DAG: SV_InstanceID + +// GLSL-DAG: main +// GLSL-DAG: gl_InstanceIndex + +// SPIRV: OpEntryPoint +// SPIRV-NOT: BuiltIn InstanceIndex +// We require 1 `Flat` for the fragment input `uint` +// SPIRV: Flat +// SPIRV-NOT: Flat + + +struct PSInput +{ + uint vInstance : SV_InstanceID; + float4 color : COLOR; +}; + +[shader("pixel")] +float4 main(PSInput input) : SV_TARGET +{ + return input.color + float(input.vInstance).xxxx; +}
\ No newline at end of file |
