Skip to content

Commit

Permalink
Enable exprs for all supported GLSL layout qualifiers (#5857)
Browse files Browse the repository at this point in the history
  • Loading branch information
@fairywreath
fairywreath authored Dec 15, 2024
1 parent 9d608b9 commit 83f4bd5
Show file tree
Hide file tree
Showing 7 changed files with 194 additions and 143 deletions.
108 changes: 66 additions & 42 deletions source/slang/slang-ast-modifier.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -263,29 +263,6 @@ class SharedModifiers : public Modifier
SLANG_AST_CLASS(SharedModifiers)
};


// A GLSL `layout` modifier
//
// We use a distinct modifier for each key that
// appears within the `layout(...)` construct,
// and each key might have an optional value token.
//
// TODO: We probably want a notion of "modifier groups"
// so that we can recover good source location info
// for modifiers that were part of the same vs.
// different constructs.
class GLSLLayoutModifier : public Modifier
{
SLANG_ABSTRACT_AST_CLASS(GLSLLayoutModifier)


// The token used to introduce the modifier is stored
// as the `nameToken` field.

// TODO: may want to accept a full expression here
Token valToken;
};

// AST nodes to represent the begin/end of a `layout` modifier group
class GLSLLayoutModifierGroupMarker : public Modifier
{
Expand All @@ -302,29 +279,12 @@ class GLSLLayoutModifierGroupEnd : public GLSLLayoutModifierGroupMarker
SLANG_AST_CLASS(GLSLLayoutModifierGroupEnd)
};


// We divide GLSL `layout` modifiers into those we have parsed
// (in the sense of having some notion of their semantics), and
// those we have not.
class GLSLParsedLayoutModifier : public GLSLLayoutModifier
{
SLANG_ABSTRACT_AST_CLASS(GLSLParsedLayoutModifier)
};

class GLSLUnparsedLayoutModifier : public GLSLLayoutModifier
class GLSLUnparsedLayoutModifier : public Modifier
{
SLANG_AST_CLASS(GLSLUnparsedLayoutModifier)
};


// Specific cases for known GLSL `layout` modifiers that we need to work with

class GLSLLocationLayoutModifier : public GLSLParsedLayoutModifier
{
SLANG_AST_CLASS(GLSLLocationLayoutModifier)
};

class GLSLBufferDataLayoutModifier : public GLSLParsedLayoutModifier
class GLSLBufferDataLayoutModifier : public Modifier
{
SLANG_AST_CLASS(GLSLBufferDataLayoutModifier)
};
Expand Down Expand Up @@ -725,6 +685,70 @@ class UncheckedGLSLOffsetLayoutAttribute : public UncheckedGLSLLayoutAttribute
SLANG_UNREFLECTED
};

class UncheckedGLSLInputAttachmentIndexLayoutAttribute : public UncheckedGLSLLayoutAttribute
{
SLANG_AST_CLASS(UncheckedGLSLInputAttachmentIndexLayoutAttribute)

SLANG_UNREFLECTED
};

class UncheckedGLSLLocationLayoutAttribute : public UncheckedGLSLLayoutAttribute
{
SLANG_AST_CLASS(UncheckedGLSLLocationLayoutAttribute)

SLANG_UNREFLECTED
};

class UncheckedGLSLIndexLayoutAttribute : public UncheckedGLSLLayoutAttribute
{
SLANG_AST_CLASS(UncheckedGLSLIndexLayoutAttribute)

SLANG_UNREFLECTED
};

class UncheckedGLSLConstantIdAttribute : public UncheckedGLSLLayoutAttribute
{
SLANG_AST_CLASS(UncheckedGLSLConstantIdAttribute)

SLANG_UNREFLECTED
};

class UncheckedGLSLRayPayloadAttribute : public UncheckedGLSLLayoutAttribute
{
SLANG_AST_CLASS(UncheckedGLSLRayPayloadAttribute)

SLANG_UNREFLECTED
};

class UncheckedGLSLRayPayloadInAttribute : public UncheckedGLSLLayoutAttribute
{
SLANG_AST_CLASS(UncheckedGLSLRayPayloadInAttribute)

SLANG_UNREFLECTED
};


class UncheckedGLSLHitObjectAttributesAttribute : public UncheckedGLSLLayoutAttribute
{
SLANG_AST_CLASS(UncheckedGLSLHitObjectAttributesAttribute)

SLANG_UNREFLECTED
};

class UncheckedGLSLCallablePayloadAttribute : public UncheckedGLSLLayoutAttribute
{
SLANG_AST_CLASS(UncheckedGLSLCallablePayloadAttribute)

SLANG_UNREFLECTED
};

class UncheckedGLSLCallablePayloadInAttribute : public UncheckedGLSLLayoutAttribute
{
SLANG_AST_CLASS(UncheckedGLSLCallablePayloadInAttribute)

SLANG_UNREFLECTED
};

// A `[name(arg0, ...)]` style attribute that has been validated.
class Attribute : public AttributeBase
{
Expand Down
2 changes: 0 additions & 2 deletions source/slang/slang-ast-print.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -434,8 +434,6 @@ void ASTPrinter::addDeclKindPrefix(Decl* decl)
continue;
if (as<RequiredGLSLExtensionModifier>(modifier))
continue;
if (as<GLSLLayoutModifier>(modifier))
continue;
if (as<GLSLLayoutModifierGroupMarker>(modifier))
continue;
if (as<HLSLLayoutSemantic>(modifier))
Expand Down
81 changes: 57 additions & 24 deletions source/slang/slang-check-modifier.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -513,6 +513,17 @@ Modifier* SemanticsVisitor::validateAttribute(

inputAttachmentIndexLayoutAttribute->location = location->getValue();
}
else if (auto locationLayoutAttr = as<GLSLLocationAttribute>(attr))
{
if (attr->args.getCount() != 1)
return nullptr;

auto location = checkConstantIntVal(attr->args[0]);
if (!location)
return nullptr;

locationLayoutAttr->value = int32_t(location->getValue());
}
else if (auto bindingAttr = as<GLSLBindingAttribute>(attr))
{
// This must be vk::binding or gl::binding (as specified in core.meta.slang under
Expand Down Expand Up @@ -1242,12 +1253,22 @@ ASTNodeType getModifierConflictGroupKind(ASTNodeType modifierType)
return ASTNodeType::OutModifier;

// Modifiers that are their own exclusive group.
case ASTNodeType::GLSLLayoutModifier:
case ASTNodeType::GLSLParsedLayoutModifier:
case ASTNodeType::GLSLLocationLayoutModifier:
case ASTNodeType::GLSLInputAttachmentIndexLayoutAttribute:
case ASTNodeType::GLSLOffsetLayoutAttribute:
case ASTNodeType::GLSLUnparsedLayoutModifier:
case ASTNodeType::UncheckedGLSLBindingLayoutAttribute:
case ASTNodeType::UncheckedGLSLSetLayoutAttribute:
case ASTNodeType::UncheckedGLSLOffsetLayoutAttribute:
case ASTNodeType::UncheckedGLSLInputAttachmentIndexLayoutAttribute:
case ASTNodeType::UncheckedGLSLLocationLayoutAttribute:
case ASTNodeType::UncheckedGLSLIndexLayoutAttribute:
case ASTNodeType::UncheckedGLSLConstantIdAttribute:
case ASTNodeType::UncheckedGLSLRayPayloadAttribute:
case ASTNodeType::UncheckedGLSLRayPayloadInAttribute:
case ASTNodeType::UncheckedGLSLHitObjectAttributesAttribute:
case ASTNodeType::UncheckedGLSLCallablePayloadAttribute:
case ASTNodeType::UncheckedGLSLCallablePayloadInAttribute:
case ASTNodeType::GLSLBufferDataLayoutModifier:
case ASTNodeType::GLSLLayoutModifierGroupMarker:
case ASTNodeType::GLSLLayoutModifierGroupBegin:
case ASTNodeType::GLSLLayoutModifierGroupEnd:
Expand Down Expand Up @@ -1321,12 +1342,10 @@ bool isModifierAllowedOnDecl(bool isGLSLInput, ASTNodeType modifierType, Decl* d
case ASTNodeType::InModifier:
case ASTNodeType::InOutModifier:
case ASTNodeType::OutModifier:
case ASTNodeType::GLSLLayoutModifier:
case ASTNodeType::GLSLParsedLayoutModifier:
case ASTNodeType::GLSLLocationLayoutModifier:
case ASTNodeType::GLSLInputAttachmentIndexLayoutAttribute:
case ASTNodeType::GLSLOffsetLayoutAttribute:
case ASTNodeType::GLSLUnparsedLayoutModifier:
case ASTNodeType::UncheckedGLSLLayoutAttribute:
case ASTNodeType::GLSLLayoutModifierGroupMarker:
case ASTNodeType::GLSLLayoutModifierGroupBegin:
case ASTNodeType::GLSLLayoutModifierGroupEnd:
Expand Down Expand Up @@ -1502,14 +1521,28 @@ AttributeBase* SemanticsVisitor::checkGLSLLayoutAttribute(

SLANG_ASSERT(uncheckedAttr->args.getCount() == 2);
}
else if (as<UncheckedGLSLOffsetLayoutAttribute>(uncheckedAttr))
{
attr = m_astBuilder->create<GLSLOffsetLayoutAttribute>();

#define CASE(UncheckedType, CheckedType) \
else if (as<UncheckedType>(uncheckedAttr)) \
{ \
attr = m_astBuilder->create<CheckedType>(); \
}

CASE(UncheckedGLSLOffsetLayoutAttribute, GLSLOffsetLayoutAttribute)
CASE(UncheckedGLSLInputAttachmentIndexLayoutAttribute, GLSLInputAttachmentIndexLayoutAttribute)
CASE(UncheckedGLSLLocationLayoutAttribute, GLSLLocationAttribute)
CASE(UncheckedGLSLIndexLayoutAttribute, GLSLIndexAttribute)
CASE(UncheckedGLSLConstantIdAttribute, VkConstantIdAttribute)
CASE(UncheckedGLSLRayPayloadAttribute, VulkanRayPayloadAttribute)
CASE(UncheckedGLSLRayPayloadInAttribute, VulkanRayPayloadInAttribute)
CASE(UncheckedGLSLHitObjectAttributesAttribute, VulkanHitObjectAttributesAttribute)
CASE(UncheckedGLSLCallablePayloadAttribute, VulkanCallablePayloadAttribute)
CASE(UncheckedGLSLCallablePayloadInAttribute, VulkanCallablePayloadInAttribute)
else
{
getSink()->diagnose(uncheckedAttr, Diagnostics::unrecognizedGLSLLayoutQualifier);
}
#undef CASE

if (attr)
{
Expand Down Expand Up @@ -1559,21 +1592,6 @@ Modifier* SemanticsVisitor::checkModifier(
return checkedAttr;
}

if (auto glslLayoutAttribute = as<UncheckedGLSLLayoutAttribute>(m))
{
return checkGLSLLayoutAttribute(glslLayoutAttribute, syntaxNode);
}

if (const auto glslImplicitOffsetAttribute = as<GLSLImplicitOffsetLayoutAttribute>(m))
{
auto offsetAttr = m_astBuilder->create<GLSLOffsetLayoutAttribute>();
offsetAttr->loc = glslImplicitOffsetAttribute->loc;

// Offset constant folding computation is deferred until all other modifiers are checked to
// ensure bindinig is checked first.
return offsetAttr;
}

if (auto decl = as<Decl>(syntaxNode))
{
auto moduleDecl = getModuleDecl(decl);
Expand All @@ -1591,6 +1609,21 @@ Modifier* SemanticsVisitor::checkModifier(
}
}

if (auto glslLayoutAttribute = as<UncheckedGLSLLayoutAttribute>(m))
{
return checkGLSLLayoutAttribute(glslLayoutAttribute, syntaxNode);
}

if (const auto glslImplicitOffsetAttribute = as<GLSLImplicitOffsetLayoutAttribute>(m))
{
auto offsetAttr = m_astBuilder->create<GLSLOffsetLayoutAttribute>();
offsetAttr->loc = glslImplicitOffsetAttribute->loc;

// Offset constant folding computation is deferred until all other modifiers are checked to
// ensure bindinig is checked first.
return offsetAttr;
}

MemoryQualifierSetModifier::Flags::MemoryQualifiersBit memoryQualifierBit =
MemoryQualifierSetModifier::Flags::kNone;
if (as<GloballyCoherentModifier>(m))
Expand Down
6 changes: 2 additions & 4 deletions source/slang/slang-lower-to-ir.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2290,14 +2290,12 @@ void addVarDecorations(IRGenContext* context, IRInst* inst, Decl* decl)
{
builder->addSimpleDecoration<IRGlobalInputDecoration>(inst);
}
else if (auto glslLocationMod = as<GLSLLocationLayoutModifier>(mod))
else if (auto glslLocationMod = as<GLSLLocationAttribute>(mod))
{
builder->addDecoration(
inst,
kIROp_GLSLLocationDecoration,
builder->getIntValue(
builder->getIntType(),
stringToInt(glslLocationMod->valToken.getContent())));
builder->getIntValue(builder->getIntType(), glslLocationMod->value));
}
else if (auto glslOffsetMod = as<GLSLOffsetLayoutAttribute>(mod))
{
Expand Down
2 changes: 1 addition & 1 deletion source/slang/slang-parameter-binding.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4250,7 +4250,7 @@ RefPtr<ProgramLayout> generateParameterBindings(TargetProgram* targetProgram, Di
{
needDefaultConstantBuffer = true;
if (varLayout->varDecl.getDecl()->hasModifier<GLSLBindingAttribute>() ||
varLayout->varDecl.getDecl()->hasModifier<GLSLLocationLayoutModifier>())
varLayout->varDecl.getDecl()->hasModifier<GLSLLocationAttribute>())
sink->diagnose(
varLayout->varDecl,
Diagnostics::explicitUniformLocation,
Expand Down
Loading

0 comments on commit 83f4bd5

Please sign in to comment.