Skip to content

Commit

Permalink
Fix spirv emit that leads to pathological downstream time. (#3546)
Browse files Browse the repository at this point in the history
  • Loading branch information
csyonghe authored Feb 3, 2024
1 parent 1476489 commit 6dca7e3
Show file tree
Hide file tree
Showing 4 changed files with 47 additions and 4 deletions.
1 change: 1 addition & 0 deletions source/slang/slang-emit-spirv.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2262,6 +2262,7 @@ struct SPIRVEmitContext
SLANG_UNIMPLEMENTED_X(e.getBuffer());
}
case kIROp_Specialize:
case kIROp_MissingReturn:
return nullptr;
case kIROp_Var:
return emitVar(parent, inst);
Expand Down
14 changes: 11 additions & 3 deletions source/slang/slang-ir-specialize-resources.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ struct ResourceParameterSpecializationCondition : FunctionCallSpecializeConditio
// our decision.
//
type = unwrapArray(type);
bool isArray = type != param->getDataType();

// On all of our (current) targets, a function that
// takes a `ConstantBuffer<T>` parameter requires
Expand All @@ -63,7 +64,7 @@ struct ResourceParameterSpecializationCondition : FunctionCallSpecializeConditio
if( isKhronosTarget(targetRequest) )
{
if (targetRequest->shouldEmitSPIRVDirectly())
return isIllegalSPIRVParameterType(type);
return isIllegalSPIRVParameterType(type, isArray);
else
return isIllegalGLSLParameterType(type);
}
Expand Down Expand Up @@ -1212,7 +1213,7 @@ bool specializeResourceUsage(

bool isIllegalGLSLParameterType(IRType* type)
{
if (as<IRUniformParameterGroupType>(type))
if (as<IRParameterGroupType>(type))
return true;
if (as<IRHLSLStructuredBufferTypeBase>(type))
return true;
Expand All @@ -1236,7 +1237,7 @@ bool isIllegalGLSLParameterType(IRType* type)
return false;
}

bool isIllegalSPIRVParameterType(IRType* type)
bool isIllegalSPIRVParameterType(IRType* type, bool isArray)
{
if (isIllegalGLSLParameterType(type))
return true;
Expand All @@ -1245,6 +1246,13 @@ bool isIllegalSPIRVParameterType(IRType* type)
// all Texture types.
if (as<IRTextureType>(type))
return true;
if (isArray)
{
if (as<IRSamplerStateTypeBase>(type))
{
return true;
}
}
return false;
}
} // namespace Slang
2 changes: 1 addition & 1 deletion source/slang/slang-ir-specialize-resources.h
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,6 @@ namespace Slang
IRModule* irModule);

bool isIllegalGLSLParameterType(IRType* type);
bool isIllegalSPIRVParameterType(IRType* type);
bool isIllegalSPIRVParameterType(IRType* type, bool isArray);

}
34 changes: 34 additions & 0 deletions tests/spirv/array-param-gep.slang
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
//TEST:SIMPLE(filecheck=CHECK): -target spirv -entry computeMain -stage compute -emit-spirv-directly

// Check that we are not generating spirv that loads a global resource array into a SSA register,
// instead, these arrays should always be accessed via direct OpAccessChain operations to avoid
// creating a lot of local load/stores in the driver compiler.

struct Scene
{
SamplerState samplers[256];
Texture2D textures[100];
}

ParameterBlock<Scene> scene;
struct Material
{
int sampler;
int texture;
}

RWStructuredBuffer<float4> result;

float4 shade(Scene scene, Material mat)
{
return scene.textures[mat.texture].SampleLevel(scene.samplers[mat.sampler], float2(0,0), 0);
}

[numthreads(1,1,1)]
void computeMain(uniform Material mat)
{
// CHECK: OpEntryPoint
// CHECK-NOT: OpLoad {{.*}} %scene{{.*}}samplers
// CHECK-NOT: OpLoad {{.*}} %scene{{.*}}textures
result[0] = shade(scene, mat);
}

0 comments on commit 6dca7e3

Please sign in to comment.