Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable exprs for GLSL binding layout qualifiers #5807

Merged
merged 10 commits into from
Dec 10, 2024
32 changes: 32 additions & 0 deletions source/slang/slang-ast-modifier.h
Original file line number Diff line number Diff line change
Expand Up @@ -693,6 +693,30 @@ class UncheckedAttribute : public AttributeBase
Scope* scope = nullptr;
};

// A GLSL layout qualifier whose value has not yet been resolved or validated.
class UncheckedGLSLLayoutAttribute : public AttributeBase
{
SLANG_AST_CLASS(UncheckedGLSLLayoutAttribute)

SLANG_UNREFLECTED
};

// GLSL `binding` layout qualifier, does not include `set`.
class UncheckedGLSLBindingLayoutAttribute : public UncheckedGLSLLayoutAttribute
{
SLANG_AST_CLASS(UncheckedGLSLBindingLayoutAttribute)

SLANG_UNREFLECTED
};

// GLSL `offset` layout qualifier.
class UncheckedGLSLOffsetLayoutAttribute : public UncheckedGLSLLayoutAttribute
{
SLANG_AST_CLASS(UncheckedGLSLOffsetLayoutAttribute)

SLANG_UNREFLECTED
};

// A `[name(arg0, ...)]` style attribute that has been validated.
class Attribute : public AttributeBase
{
Expand Down Expand Up @@ -854,6 +878,14 @@ class GLSLOffsetLayoutAttribute : public Attribute
int64_t offset;
};

// Implicitly added offset qualifier when no offset is specified.
class GLSLImplicitOffsetLayoutAttribute : public AttributeBase
{
SLANG_AST_CLASS(GLSLImplicitOffsetLayoutAttribute)

SLANG_UNREFLECTED
};

class GLSLSimpleIntegerLayoutAttribute : public Attribute
{
SLANG_AST_CLASS(GLSLSimpleIntegerLayoutAttribute)
Expand Down
24 changes: 24 additions & 0 deletions source/slang/slang-check-impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -614,6 +614,17 @@ struct ImplicitCastMethodKey
}
};

/// Used to track offsets for atomic counter storage qualifiers.
struct GLSLBindingOffsetTracker
{
public:
void setBindingOffset(int binding, int64_t byteOffset);
int64_t getNextBindingOffset(int binding);

private:
Dictionary<int, int64_t> bindingToByteOffset;
};

/// Shared state for a semantics-checking session.
struct SharedSemanticsContext : public RefObject
{
Expand Down Expand Up @@ -645,6 +656,8 @@ struct SharedSemanticsContext : public RefObject
List<ModuleDecl*> importedModulesList;
HashSet<ModuleDecl*> importedModulesSet;

GLSLBindingOffsetTracker m_glslBindingOffsetTracker;

public:
SharedSemanticsContext(
Linkage* linkage,
Expand Down Expand Up @@ -705,6 +718,8 @@ struct SharedSemanticsContext : public RefObject
InheritanceCircularityInfo* next = nullptr;
};

GLSLBindingOffsetTracker* getGLSLBindingOffsetTracker() { return &m_glslBindingOffsetTracker; }

/// Get the processed inheritance information for `type`, including all its facets
InheritanceInfo getInheritanceInfo(
Type* type,
Expand Down Expand Up @@ -1055,6 +1070,11 @@ struct SemanticsContext

OrderedHashSet<Type*>* getCapturedTypePacks() { return m_capturedTypePacks; }

GLSLBindingOffsetTracker* getGLSLBindingOffsetTracker()
{
return m_shared->getGLSLBindingOffsetTracker();
}

private:
SharedSemanticsContext* m_shared = nullptr;

Expand Down Expand Up @@ -1647,6 +1667,10 @@ struct SemanticsVisitor : public SemanticsContext
UncheckedAttribute* uncheckedAttr,
ModifiableSyntaxNode* attrTarget);

AttributeBase* checkGLSLLayoutAttribute(
UncheckedGLSLLayoutAttribute* uncheckedAttr,
ModifiableSyntaxNode* attrTarget);

Modifier* checkModifier(
Modifier* m,
ModifiableSyntaxNode* syntaxNode,
Expand Down
159 changes: 153 additions & 6 deletions source/slang/slang-check-modifier.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ ConstantIntVal* SemanticsVisitor::checkConstantIntVal(Expr* expr)
IntegerConstantExpressionCoercionType::AnyInteger,
nullptr,
ConstantFoldingKind::CompileTime);

if (!intVal)
return nullptr;

Expand Down Expand Up @@ -524,17 +525,32 @@ Modifier* SemanticsVisitor::validateAttribute(
}

// TODO(JS): Prior validation currently doesn't ensure both args are ints (as specified in
// core.meta.slang), so check here to make sure they both are
auto binding = checkConstantIntVal(attr->args[0]);
auto set = checkConstantIntVal(attr->args[1]);
// core.meta.slang), so check here to make sure they both are.
//
// Binding attribute may also be created from GLSL style layout qualifiers where only one of
// the args are specified, hence check for each individually.
ConstantIntVal* binding = nullptr;
if (attr->args[0])
binding = checkConstantIntVal(attr->args[0]);

ConstantIntVal* set = nullptr;
if (attr->args[1])
set = checkConstantIntVal(attr->args[1]);

if (binding == nullptr || set == nullptr)
if (!binding && !set)
{
return nullptr;
}

bindingAttr->binding = int32_t(binding->getValue());
bindingAttr->set = int32_t(set->getValue());
if (binding)
{
bindingAttr->binding = int32_t(binding->getValue());
}

if (set)
{
bindingAttr->set = int32_t(set->getValue());
}
}
else if (auto simpleLayoutAttr = as<GLSLSimpleIntegerLayoutAttribute>(attr))
{
Expand Down Expand Up @@ -1430,6 +1446,97 @@ bool isModifierAllowedOnDecl(bool isGLSLInput, ASTNodeType modifierType, Decl* d
}
}

void GLSLBindingOffsetTracker::setBindingOffset(int binding, int64_t byteOffset)
{
bindingToByteOffset.set(binding, byteOffset);
}

int64_t GLSLBindingOffsetTracker::getNextBindingOffset(int binding)
{
int64_t currentOffset;
if (bindingToByteOffset.addIfNotExists(binding, 0))
currentOffset = 0;
else
currentOffset = bindingToByteOffset.getValue(binding) + sizeof(uint32_t);

bindingToByteOffset.set(binding, currentOffset + sizeof(uint32_t));
return currentOffset;
}
fairywreath marked this conversation as resolved.
Show resolved Hide resolved

AttributeBase* SemanticsVisitor::checkGLSLLayoutAttribute(
UncheckedGLSLLayoutAttribute* uncheckedAttr,
ModifiableSyntaxNode* attrTarget)
{
SLANG_ASSERT(uncheckedAttr->args.getCount() == 1);

Attribute* attr = nullptr;

// False if the current unchecked attribute node is deleted and does not result in a new checked
// attribute.
bool addNode = true;

const auto& name = uncheckedAttr->getKeywordName()->text;
if (as<UncheckedGLSLBindingLayoutAttribute>(uncheckedAttr) || name == "set")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

May as well add a type for UncheckedGLSLSetLayoutAttribute and get rid of the name comparison here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I will clean this up in another patch that implements the other GLSL layout modifiers to exprs. I originally did not want to add a new Unchecked AST modifier node for every modifier but this is indeed cleaner.

{
// Binding and set are coupled together as a descriptor table slot resource for codegen.
// Attempt to retrieve and annotate an existing binding attribute or create a new one.
attr = attrTarget->findModifier<GLSLBindingAttribute>();
if (!attr)
{
attr = m_astBuilder->create<GLSLBindingAttribute>();
}
else
{
addNode = false;
}

// `validateAttribute`, which will be called to parse the binding arguments, also accepts
// modifiers from vk::binding and gl::binding where both set and binding are specified.
// Binding is the first and set is the second argument - specify them explicitly here.
if (as<UncheckedGLSLBindingLayoutAttribute>(uncheckedAttr))
{
uncheckedAttr->args.add(nullptr);
}
else
{
uncheckedAttr->args.add(uncheckedAttr->args[0]);
uncheckedAttr->args[0] = nullptr;
}

SLANG_ASSERT(uncheckedAttr->args.getCount() == 2);
}
else if (as<UncheckedGLSLOffsetLayoutAttribute>(uncheckedAttr))
{
attr = m_astBuilder->create<GLSLOffsetLayoutAttribute>();
}
else
{
getSink()->diagnose(uncheckedAttr, Diagnostics::unrecognizedGLSLLayoutQualifier);
}

if (attr)
{
attr->keywordName = uncheckedAttr->keywordName;
attr->originalIdentifierToken = uncheckedAttr->originalIdentifierToken;
attr->args = uncheckedAttr->args;
attr->loc = uncheckedAttr->loc;

// Offset constant folding computation is deferred until all other modifiers are checked to
// ensure bindinig is checked first.
if (!as<GLSLOffsetLayoutAttribute>(attr))
{
validateAttribute(attr, nullptr, attrTarget);
fairywreath marked this conversation as resolved.
Show resolved Hide resolved
}
}

if (!addNode)
{
attr = nullptr;
}

return attr;
}

Modifier* SemanticsVisitor::checkModifier(
Modifier* m,
ModifiableSyntaxNode* syntaxNode,
Expand All @@ -1455,6 +1562,21 @@ 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 Down Expand Up @@ -1903,6 +2025,31 @@ void SemanticsVisitor::checkModifiers(ModifiableSyntaxNode* syntaxNode)
// install the new list of modifiers on the declaration
syntaxNode->modifiers.first = resultModifiers;

// GLSL offset layout qualifiers are resolved after all other modifiers are checked to ensure
// binding layout qualifiers are processed first.
if (auto glslOffsetAttribute = syntaxNode->findModifier<GLSLOffsetLayoutAttribute>())
{
if (const auto glslBindingAttribute = syntaxNode->findModifier<GLSLBindingAttribute>())
{
if (glslOffsetAttribute->args.getCount() == 0)
{
glslOffsetAttribute->offset = getGLSLBindingOffsetTracker()->getNextBindingOffset(
glslBindingAttribute->binding);
}
else if (const auto constVal = checkConstantIntVal(glslOffsetAttribute->args[0]))
{
fairywreath marked this conversation as resolved.
Show resolved Hide resolved
glslOffsetAttribute->offset = uint64_t(constVal->getValue());
getGLSLBindingOffsetTracker()->setBindingOffset(
glslBindingAttribute->binding,
glslOffsetAttribute->offset);
}
}
else
{
getSink()->diagnose(glslOffsetAttribute, Diagnostics::missingLayoutBindingModifier);
}
}

postProcessingOnModifiers(syntaxNode->modifiers);
}

Expand Down
7 changes: 7 additions & 0 deletions source/slang/slang-diagnostic-defs.h
Original file line number Diff line number Diff line change
Expand Up @@ -1207,6 +1207,13 @@ DIAGNOSTIC(
Error,
cannotUseUnsizedTypeInConstantBuffer,
"cannot use unsized type '$0' in a constant buffer.")
DIAGNOSTIC(31216, Error, unrecognizedGLSLLayoutQualifier, "GLSL layout qualifier is unrecognized")
DIAGNOSTIC(
31217,
Error,
unrecognizedGLSLLayoutQualifierOrRequiresAssignment,
"GLSL layout qualifier is unrecognized or requires assignment")


// Enums

Expand Down
Loading
Loading