Skip to content

Commit

Permalink
Properly plumbing layout for global varyings. (#6198)
Browse files Browse the repository at this point in the history
* Properly plumbing layout for global varyings.

* Fix test.
  • Loading branch information
csyonghe authored Jan 28, 2025
1 parent da3dc98 commit f030a5a
Show file tree
Hide file tree
Showing 6 changed files with 106 additions and 31 deletions.
39 changes: 21 additions & 18 deletions source/slang/slang-check-decl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -842,26 +842,29 @@ bool isGlobalShaderParameter(VarDeclBase* decl)
if (!isGlobalDecl(decl))
return false;

// A global variable marked `static` indicates a traditional
// global variable (albeit one that is implicitly local to
// the translation unit)
//
if (decl->hasModifier<HLSLStaticModifier>())
return false;
for (auto modifier : decl->modifiers)
{
// A global variable marked `static` indicates a traditional
// global variable (albeit one that is implicitly local to
// the translation unit)
//
if (as<HLSLStaticModifier>(modifier))
return false;

// While not normally allowed, out variables are not constant
// parameters, this can happen for example in GLSL mode
if (decl->hasModifier<OutModifier>())
return false;
if (decl->hasModifier<InModifier>())
return false;
// While not normally allowed, out variables are not constant
// parameters, this can happen for example in GLSL mode
if (as<OutModifier>(modifier))
return false;
if (as<InModifier>(modifier))
return false;

// The `groupshared` modifier indicates that a variable cannot
// be a shader parameters, but is instead transient storage
// allocated for the duration of a thread-group's execution.
//
if (decl->hasModifier<HLSLGroupSharedModifier>())
return false;
// The `groupshared` modifier indicates that a variable cannot
// be a shader parameters, but is instead transient storage
// allocated for the duration of a thread-group's execution.
//
if (as<HLSLGroupSharedModifier>(modifier))
return false;
}

return true;
}
Expand Down
14 changes: 13 additions & 1 deletion source/slang/slang-check-shader.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -741,7 +741,19 @@ void Module::_collectShaderParams()
// things like `static` globals and `groupshared` variables.
//
if (!isGlobalShaderParameter(globalVar))
continue;
{
bool isVarying = false;
for (auto m : globalVar->modifiers)
{
if (as<InModifier>(m) || as<OutModifier>(m))
{
isVarying = true;
break;
}
}
if (!isVarying)
continue;
}

// At this point we know we have a global shader parameter.

Expand Down
29 changes: 18 additions & 11 deletions source/slang/slang-ir-translate-glsl-global-var.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -86,11 +86,6 @@ struct GlobalVarTranslationContext
IRTypeLayout::Builder fieldTypeLayout(&builder);
IRVarLayout::Builder varLayoutBuilder(&builder, fieldTypeLayout.build());
varLayoutBuilder.setStage(entryPointDecor->getProfile().getStage());
if (auto locationDecoration = input->findDecoration<IRGLSLLocationDecoration>())
{
varLayoutBuilder.findOrAddResourceInfo(LayoutResourceKind::VaryingInput)
->offset = (UInt)getIntVal(locationDecoration->getLocation());
}
if (auto semanticDecor = input->findDecoration<IRSemanticDecoration>())
{
varLayoutBuilder.setSystemValueSemantic(
Expand All @@ -102,6 +97,16 @@ struct GlobalVarTranslationContext
fieldTypeLayout.addResourceUsage(
LayoutResourceKind::VaryingInput,
LayoutSize(1));
if (auto layoutDecor = findVarLayout(input))
{
if (auto offsetAttr =
layoutDecor->findOffsetAttr(LayoutResourceKind::VaryingInput))
{
varLayoutBuilder
.findOrAddResourceInfo(LayoutResourceKind::VaryingInput)
->offset = (UInt)offsetAttr->getOffset();
}
}
if (entryPointDecor->getProfile().getStage() == Stage::Fragment)
{
varLayoutBuilder.setUserSemantic("COLOR", inputVarIndex);
Expand Down Expand Up @@ -180,13 +185,15 @@ struct GlobalVarTranslationContext
fieldTypeLayout.addResourceUsage(
LayoutResourceKind::VaryingOutput,
LayoutSize(1));

if (auto locationDecoration =
output->findDecoration<IRGLSLLocationDecoration>())
if (auto layoutDecor = findVarLayout(output))
{
varLayoutBuilder
.findOrAddResourceInfo(LayoutResourceKind::VaryingOutput)
->offset = (UInt)getIntVal(locationDecoration->getLocation());
if (auto offsetAttr =
layoutDecor->findOffsetAttr(LayoutResourceKind::VaryingOutput))
{
varLayoutBuilder
.findOrAddResourceInfo(LayoutResourceKind::VaryingOutput)
->offset = (UInt)offsetAttr->getOffset();
}
}
if (entryPointDecor->getProfile().getStage() == Stage::Fragment)
{
Expand Down
4 changes: 3 additions & 1 deletion source/slang/slang-lower-to-ir.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -12209,7 +12209,9 @@ RefPtr<IRModule> TargetProgram::createIRModuleForLayout(DiagnosticSink* sink)
// has been emitted to this module, so that we will have something
// to decorate.
//
auto irVar = getSimpleVal(context, ensureDecl(context, varDecl.getDecl()));
auto irVar = materialize(context, ensureDecl(context, varDecl.getDecl())).val;
if (!irVar)
SLANG_UNEXPECTED("unhandled value flavor");

auto irLayout = lowerVarLayout(context, varLayout);

Expand Down
28 changes: 28 additions & 0 deletions source/slang/slang-parameter-binding.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -712,6 +712,15 @@ RefPtr<TypeLayout> getTypeLayoutForGlobalShaderParameter(
return createTypeLayoutWith(layoutContext, specializationConstantRule, type);
}

if (varDecl->hasModifier<InModifier>())
{
return createTypeLayoutWith(layoutContext, rules->getVaryingInputRules(), type);
}
else if (varDecl->hasModifier<OutModifier>())
{
return createTypeLayoutWith(layoutContext, rules->getVaryingOutputRules(), type);
}

// TODO(tfoley): there may be other cases that we need to handle here

// An "ordinary" global variable is implicitly a uniform
Expand Down Expand Up @@ -1120,6 +1129,25 @@ static void addExplicitParameterBindings_GLSL(
return;
}

if (auto foundVaryingInput = typeLayout->FindResourceInfo(LayoutResourceKind::VaryingInput))
{
info[kResInfo].resInfo = foundVaryingInput;

if (auto layoutAttr = varDecl.getDecl()->findModifier<GLSLLocationAttribute>())
info[kResInfo].semanticInfo.index = layoutAttr->value;
else
return;
}
if (auto foundVaryingOutput = typeLayout->FindResourceInfo(LayoutResourceKind::VaryingOutput))
{
info[kResInfo].resInfo = foundVaryingOutput;

if (auto layoutAttr = varDecl.getDecl()->findModifier<GLSLLocationAttribute>())
info[kResInfo].semanticInfo.index = layoutAttr->value;
else
return;
}

// For remaining cases, we only want to apply GLSL-style layout modifers
// when compiling for Khronos and WGSL targets.
//
Expand Down
23 changes: 23 additions & 0 deletions tests/glsl/in-layout.slang
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
//TEST:SIMPLE(filecheck=CHECK): -target spirv -entry main -stage fragment

// Test that we can mix explicit and implicit locations for global varyings.

#version 450
layout(location=1) in vec4 color_0;
in vec2 texcoord_0;
in vec2 texCoord_1;

out vec4 out1;

// CHECK-DAG: OpDecorate %color_0 Location 1

// CHECK-DAG: OpDecorate %texcoord_0 Location 0

// CHECK-DAG: OpDecorate %texCoord_1 Location 2

// CHECK-DAG: OpDecorate %entryPointParam_main_out1 Location 0

void main()
{
out1 = vec4(color_0.rg, texcoord_0 + texCoord_1);
}

0 comments on commit f030a5a

Please sign in to comment.