summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
-rw-r--r--source/slang/compiler.h6
-rw-r--r--source/slang/diagnostic-defs.h3
-rw-r--r--source/slang/parameter-binding.cpp131
-rw-r--r--source/slang/type-layout.cpp32
-rw-r--r--tests/diagnostics/vk-bindings.slang16
-rw-r--r--tests/diagnostics/vk-bindings.slang.expected7
-rw-r--r--tests/reflection/parameter-block-explicit-space.slang104
-rw-r--r--tests/reflection/parameter-block-explicit-space.slang.expected103
8 files changed, 359 insertions, 43 deletions
diff --git a/source/slang/compiler.h b/source/slang/compiler.h
index 7be5b332d..fbd6b3f15 100644
--- a/source/slang/compiler.h
+++ b/source/slang/compiler.h
@@ -241,6 +241,12 @@ namespace Slang
MatrixLayoutMode getDefaultMatrixLayoutMode();
};
+ /// Are we generating code for a D3D API?
+ bool isD3DTarget(TargetRequest* targetReq);
+
+ /// Are we generating code for a Khronos API (OpenGL or Vulkan)?
+ bool isKhronosTarget(TargetRequest* targetReq);
+
// Compute the "effective" profile to use when outputting the given entry point
// for the chosen code-generation target.
//
diff --git a/source/slang/diagnostic-defs.h b/source/slang/diagnostic-defs.h
index 8466485e7..2a08dfb6a 100644
--- a/source/slang/diagnostic-defs.h
+++ b/source/slang/diagnostic-defs.h
@@ -368,6 +368,9 @@ DIAGNOSTIC(39009, Error, expectedSpace, "expected 'space', got '$0'")
DIAGNOSTIC(39010, Error, expectedSpaceIndex, "expected a register space index after 'space'")
DIAGNOSTIC(39011, Error, componentMaskNotSupported, "explicit register component masks are not yet supported in Slang")
DIAGNOSTIC(39012, Error, packOffsetNotSupported, "explicit 'packoffset' bindings are not yet supported in Slang")
+DIAGNOSTIC(39013, Warning, registerModifierButNoVulkanLayout, "shader parameter '$0' has a 'register' specified for D3D, but no '[[vk::binding(...)]]` specified for Vulkan")
+DIAGNOSTIC(39014, Error, unexpectedSpecifierAfterSpace, "unexpected specifier after register space: '$0'")
+DIAGNOSTIC(39015, Error, wholeSpaceParameterRequiresZeroBinding, "shader parameter '$0' consumes whole descriptor sets, so the binding must be in the form '[[vk::binding(0, ...)]]'; the non-zero binding '$1' is not allowed")
DIAGNOSTIC(39013, Error, dontExpectOutParametersForStage, "the '$0' stage does not support `out` or `inout` entry point parameters")
DIAGNOSTIC(39014, Error, dontExpectInParametersForStage, "the '$0' stage does not support `in` entry point parameters")
diff --git a/source/slang/parameter-binding.cpp b/source/slang/parameter-binding.cpp
index bc0f5b760..58891a956 100644
--- a/source/slang/parameter-binding.cpp
+++ b/source/slang/parameter-binding.cpp
@@ -338,7 +338,7 @@ struct SharedParameterBindingContext
// TODO: We should eventually strip this down to
// just the subset of fields on the target that
// can influence layout decisions.
- TargetRequest* targetRequest;
+ TargetRequest* targetRequest = nullptr;
LayoutRulesFamilyImpl* defaultLayoutRules;
@@ -443,6 +443,36 @@ static void splitNameAndIndex(
outDigits = UnownedStringSlice(digitsBegin, digitsEnd);
}
+LayoutResourceKind findRegisterClassFromName(UnownedStringSlice const& registerClassName)
+{
+ switch( registerClassName.size() )
+ {
+ case 1:
+ switch (*registerClassName.begin())
+ {
+ case 'b': return LayoutResourceKind::ConstantBuffer;
+ case 't': return LayoutResourceKind::ShaderResource;
+ case 'u': return LayoutResourceKind::UnorderedAccess;
+ case 's': return LayoutResourceKind::SamplerState;
+
+ default:
+ break;
+ }
+ break;
+
+ case 5:
+ if( registerClassName == "space" )
+ {
+ return LayoutResourceKind::RegisterSpace;
+ }
+ break;
+
+ default:
+ break;
+ }
+ return LayoutResourceKind::None;
+}
+
LayoutSemanticInfo ExtractLayoutSemanticInfo(
ParameterBindingContext* context,
HLSLLayoutSemantic* semantic)
@@ -470,31 +500,9 @@ LayoutSemanticInfo ExtractLayoutSemanticInfo(
UnownedStringSlice registerIndexDigits;
splitNameAndIndex(registerName, registerClassName, registerIndexDigits);
- // All of the register classes we support are single ASCII characters,
- // so we really just care about the first byte, but we want to be
- // careful and only look at it if the register class name is one
- // byte long.
- char registerClassChar = registerClassName.size() == 1 ? *registerClassName.begin() : 0;
- LayoutResourceKind kind = LayoutResourceKind::None;
- switch (registerClassChar)
+ LayoutResourceKind kind = findRegisterClassFromName(registerClassName);
+ if(kind == LayoutResourceKind::None)
{
- case 'b':
- kind = LayoutResourceKind::ConstantBuffer;
- break;
-
- case 't':
- kind = LayoutResourceKind::ShaderResource;
- break;
-
- case 'u':
- kind = LayoutResourceKind::UnorderedAccess;
- break;
-
- case 's':
- kind = LayoutResourceKind::SamplerState;
- break;
-
- default:
getSink(context)->diagnose(semantic->registerName, Diagnostics::unknownRegisterClass, registerClassName);
return info;
}
@@ -524,13 +532,17 @@ LayoutSemanticInfo ExtractLayoutSemanticInfo(
UnownedStringSlice spaceDigits;
splitNameAndIndex(spaceName, spaceSpelling, spaceDigits);
- if( spaceSpelling != UnownedTerminatedStringSlice("space") )
+ if( kind == LayoutResourceKind::RegisterSpace )
+ {
+ getSink(context)->diagnose(registerSemantic->spaceName, Diagnostics::unexpectedSpecifierAfterSpace, spaceName);
+ }
+ else if( spaceSpelling != UnownedTerminatedStringSlice("space") )
{
- getSink(context)->diagnose(semantic->registerName, Diagnostics::expectedSpace, spaceSpelling);
+ getSink(context)->diagnose(registerSemantic->spaceName, Diagnostics::expectedSpace, spaceSpelling);
}
else if( spaceDigits.size() == 0 )
{
- getSink(context)->diagnose(semantic->registerName, Diagnostics::expectedSpaceIndex);
+ getSink(context)->diagnose(registerSemantic->spaceName, Diagnostics::expectedSpaceIndex);
}
else
{
@@ -1491,6 +1503,26 @@ static void addExplicitParameterBindings_HLSL(
RefPtr<ParameterInfo> parameterInfo,
RefPtr<VarLayout> varLayout)
{
+ // We only want to apply D3D `register` modifiers when compiling for
+ // D3D targets.
+ //
+ // TODO: Nominally, the `register` keyword allows for a shader
+ // profile to be specified, so that a given binding only
+ // applies for a specific profile:
+ //
+ // https://docs.microsoft.com/en-us/windows/desktop/direct3dhlsl/dx-graphics-hlsl-variable-register
+ //
+ // We might want to consider supporting that syntax in the
+ // long run, in order to handle bindings for multiple targets
+ // in a more consistent fashion (whereas using `register` for D3D
+ // and `[[vk::binding(...)]]` for Vulkan creates a lot of
+ // visual noise).
+ //
+ // For now we do the filtering on target in a very direct fashion:
+ //
+ if(!isD3DTarget(context->getTargetRequest()))
+ return;
+
auto typeLayout = varLayout->typeLayout;
auto varDecl = varLayout->varDecl;
@@ -1527,11 +1559,35 @@ static void addExplicitParameterBindings_HLSL(
}
}
+static void maybeDiagnoseMissingVulkanLayoutModifier(
+ ParameterBindingContext* context,
+ DeclRef<VarDeclBase> const& varDecl)
+{
+ // If the user didn't specify a `binding` (and optional `set`) for Vulkan,
+ // but they *did* specify a `register` for D3D, then that is probably an
+ // oversight on their part.
+ if( auto registerModifier = varDecl.getDecl()->FindModifier<HLSLRegisterSemantic>() )
+ {
+ getSink(context)->diagnose(registerModifier, Diagnostics::registerModifierButNoVulkanLayout, varDecl.GetName());
+ }
+}
+
static void addExplicitParameterBindings_GLSL(
ParameterBindingContext* context,
RefPtr<ParameterInfo> parameterInfo,
RefPtr<VarLayout> varLayout)
{
+
+ // We only want to apply GLSL-style layout modifers
+ // when compiling for a Khronos-related target.
+ //
+ // TODO: This should have some finer granularity
+ // so that we are able to distinguish between
+ // Vulkan and OpenGL as targets.
+ //
+ if(!isKhronosTarget(context->getTargetRequest()))
+ return;
+
auto typeLayout = varLayout->typeLayout;
auto varDecl = varLayout->varDecl;
@@ -1556,11 +1612,28 @@ static void addExplicitParameterBindings_GLSL(
auto attr = varDecl.getDecl()->FindModifier<GLSLBindingAttribute>();
if (!attr)
{
+ maybeDiagnoseMissingVulkanLayoutModifier(context, varDecl);
return;
}
semanticInfo.index = attr->binding;
semanticInfo.space = attr->set;
}
+ else if( (resInfo = typeLayout->FindResourceInfo(LayoutResourceKind::RegisterSpace)) != nullptr )
+ {
+ // Try to find `set`
+ auto attr = varDecl.getDecl()->FindModifier<GLSLBindingAttribute>();
+ if (!attr)
+ {
+ maybeDiagnoseMissingVulkanLayoutModifier(context, varDecl);
+ return;
+ }
+ if( attr->binding != 0)
+ {
+ getSink(context)->diagnose(attr, Diagnostics::wholeSpaceParameterRequiresZeroBinding, varDecl.GetName(), attr->binding);
+ }
+ semanticInfo.index = attr->set;
+ semanticInfo.space = 0;
+ }
else if( (resInfo = typeLayout->FindResourceInfo(LayoutResourceKind::VertexInput)) != nullptr )
{
// Try to find `location` binding
@@ -2564,6 +2637,7 @@ void generateParameterBindings(
sharedContext.compileRequest = compileReq;
sharedContext.defaultLayoutRules = layoutContext.getRulesFamily();
sharedContext.programLayout = programLayout;
+ sharedContext.targetRequest = targetReq;
// Create a sub-context to collect parameters that get
// declared into the global scope
@@ -2816,6 +2890,7 @@ RefPtr<ProgramLayout> specializeProgramLayout(
sharedContext.compileRequest = targetReq->compileRequest;
sharedContext.defaultLayoutRules = layoutContext.getRulesFamily();
sharedContext.programLayout = newProgramLayout;
+ sharedContext.targetRequest = targetReq;
// Create a sub-context to collect parameters that get
// declared into the global scope
diff --git a/source/slang/type-layout.cpp b/source/slang/type-layout.cpp
index 41476a7e5..4e9a05b53 100644
--- a/source/slang/type-layout.cpp
+++ b/source/slang/type-layout.cpp
@@ -823,7 +823,7 @@ static bool isOpenGLTarget(TargetRequest*)
return false;
}
-static bool isD3DTarget(TargetRequest* targetReq)
+bool isD3DTarget(TargetRequest* targetReq)
{
switch( targetReq->target )
{
@@ -839,6 +839,20 @@ static bool isD3DTarget(TargetRequest* targetReq)
}
}
+bool isKhronosTarget(TargetRequest* targetReq)
+{
+ switch( targetReq->target )
+ {
+ default:
+ return false;
+
+ case CodeGenTarget::GLSL:
+ case CodeGenTarget::SPIRV:
+ case CodeGenTarget::SPIRVAssembly:
+ return true;
+ }
+}
+
static bool isD3D11Target(TargetRequest*)
{
// We aren't officially supporting D3D11 right now
@@ -886,21 +900,9 @@ static bool isSM5_1OrLater(TargetRequest* targetReq)
static bool isVulkanTarget(TargetRequest* targetReq)
{
- switch( targetReq->target )
- {
- default:
- return false;
-
- case CodeGenTarget::GLSL:
- case CodeGenTarget::SPIRV:
- case CodeGenTarget::SPIRVAssembly:
- break;
- }
-
- // For right now, any GLSL-related target is assumed
+ // For right now, any Khronos-related target is assumed
// to be a Vulkan target.
-
- return true;
+ return isKhronosTarget(targetReq);
}
static bool shouldAllocateRegisterSpaceForParameterBlock(
diff --git a/tests/diagnostics/vk-bindings.slang b/tests/diagnostics/vk-bindings.slang
new file mode 100644
index 000000000..f5a8bdeba
--- /dev/null
+++ b/tests/diagnostics/vk-bindings.slang
@@ -0,0 +1,16 @@
+// vk-bindings.slang
+
+//TEST:SIMPLE:-target spirv
+
+// D3D `register` without VK binding
+Texture2D t : register(t0);
+
+struct S { float4 a; };
+
+// Parameter block with non-zero binding:
+[[vk::binding(2,1)]]
+ParameterBlock<S> b;
+
+[shader("compute")]
+void main()
+{} \ No newline at end of file
diff --git a/tests/diagnostics/vk-bindings.slang.expected b/tests/diagnostics/vk-bindings.slang.expected
new file mode 100644
index 000000000..8e085b55f
--- /dev/null
+++ b/tests/diagnostics/vk-bindings.slang.expected
@@ -0,0 +1,7 @@
+result code = -1
+standard error = {
+tests/diagnostics/vk-bindings.slang(6): warning 39013: shader parameter 't' has a 'register' specified for D3D, but no '[[vk::binding(...)]]` specified for Vulkan
+tests/diagnostics/vk-bindings.slang(11): error 39015: shader parameter 'b' consumes whole descriptor sets, so the binding must be in the form '[[vk::binding(0, ...)]]'; the non-zero binding '2' is not allowed
+}
+standard output = {
+}
diff --git a/tests/reflection/parameter-block-explicit-space.slang b/tests/reflection/parameter-block-explicit-space.slang
new file mode 100644
index 000000000..5679a1c35
--- /dev/null
+++ b/tests/reflection/parameter-block-explicit-space.slang
@@ -0,0 +1,104 @@
+// parameter-block-explicit-space.slang
+
+//TEST:REFLECTION:-D__SLANG__ -stage fragment -entry main -profile sm_5_1 -target hlsl
+//TEST:COMPARE_HLSL:-stage fragment -entry main -profile sm_5_1
+
+#ifdef __SLANG__
+struct A
+{
+ float4 au;
+ Texture2D at1;
+ Texture2D at2;
+ SamplerState as;
+}
+
+struct X
+{
+ float4 xu;
+}
+
+struct B
+{
+ float4 bu;
+ Texture2D bt;
+ SamplerState bs;
+
+ // TODO: This line leads to a crash
+// ConstantBuffer<X> bx;
+}
+
+
+
+[[vk::binding(0,2)]]
+ParameterBlock<A> a : register(space2);
+
+[[vk::binding(0,3)]]
+ParameterBlock<B> b : register(space3);
+
+float4 use(float4 val) { return val; }
+float4 use(Texture2D t, SamplerState s)
+{
+ return t.Sample(s, 0.0);
+}
+
+float4 main() : SV_Target
+{
+ return use(a.au)
+ + use(a.at1, a.as)
+ + use(a.at2, a.as)
+ + use(b.bu)
+ + use(b.bt, b.bs)
+ // + use(b.bx.xu)
+ ;
+}
+
+#else
+
+#define A A_0
+#define a a_0
+#define au au_0
+#define at1 a_at1_0
+#define at2 a_at2_0
+#define as a_as_0
+
+#define B B_0
+#define b b_0
+#define bu bu_0
+#define bt b_bt_0
+#define bs b_bs_0
+
+struct A
+{
+ float4 au;
+};
+cbuffer _S1 : register(b0, space2)
+{ A a; }
+Texture2D at1 : register(t0, space2);
+Texture2D at2 : register(t1, space2);
+SamplerState as : register(s0, space2);
+
+struct B
+{
+ float4 bu;
+};
+cbuffer _S3 : register(b0, space3)
+{ B b; }
+Texture2D bt : register(t0, space3);
+SamplerState bs : register(s0, space3);
+
+float4 use(float4 val) { return val; }
+float4 use(Texture2D t, SamplerState s)
+{
+ return t.Sample(s, 0.0);
+}
+
+float4 main() : SV_TARGET
+{
+ return use(a.au)
+ + use(at1, as)
+ + use(at2, as)
+ + use(b.bu)
+ + use(bt, bs);
+}
+
+#endif
diff --git a/tests/reflection/parameter-block-explicit-space.slang.expected b/tests/reflection/parameter-block-explicit-space.slang.expected
new file mode 100644
index 000000000..e683a641f
--- /dev/null
+++ b/tests/reflection/parameter-block-explicit-space.slang.expected
@@ -0,0 +1,103 @@
+result code = 0
+standard error = {
+}
+standard output = {
+{
+ "parameters": [
+ {
+ "name": "a",
+ "binding": {"kind": "constantBuffer", "space": 2, "index": 0, "count": 0},
+ "type": {
+ "kind": "parameterBlock",
+ "elementType": {
+ "kind": "struct",
+ "name": "A",
+ "fields": [
+ {
+ "name": "au",
+ "type": {
+ "kind": "vector",
+ "elementCount": 4,
+ "elementType": {
+ "kind": "scalar",
+ "scalarType": "float32"
+ }
+ },
+ "binding": {"kind": "uniform", "offset": 0, "size": 16}
+ },
+ {
+ "name": "at1",
+ "type": {
+ "kind": "resource",
+ "baseShape": "texture2D"
+ },
+ "binding": {"kind": "shaderResource", "index": 0}
+ },
+ {
+ "name": "at2",
+ "type": {
+ "kind": "resource",
+ "baseShape": "texture2D"
+ },
+ "binding": {"kind": "shaderResource", "index": 1}
+ },
+ {
+ "name": "as",
+ "type": {
+ "kind": "samplerState"
+ },
+ "binding": {"kind": "samplerState", "index": 0}
+ }
+ ]
+ }
+ }
+ },
+ {
+ "name": "b",
+ "binding": {"kind": "constantBuffer", "space": 3, "index": 0, "count": 0},
+ "type": {
+ "kind": "parameterBlock",
+ "elementType": {
+ "kind": "struct",
+ "name": "B",
+ "fields": [
+ {
+ "name": "bu",
+ "type": {
+ "kind": "vector",
+ "elementCount": 4,
+ "elementType": {
+ "kind": "scalar",
+ "scalarType": "float32"
+ }
+ },
+ "binding": {"kind": "uniform", "offset": 0, "size": 16}
+ },
+ {
+ "name": "bt",
+ "type": {
+ "kind": "resource",
+ "baseShape": "texture2D"
+ },
+ "binding": {"kind": "shaderResource", "index": 0}
+ },
+ {
+ "name": "bs",
+ "type": {
+ "kind": "samplerState"
+ },
+ "binding": {"kind": "samplerState", "index": 0}
+ }
+ ]
+ }
+ }
+ }
+ ],
+ "entryPoints": [
+ {
+ "name": "main",
+ "stage:": "fragment"
+ }
+ ]
+}
+}