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
30 changes: 30 additions & 0 deletions source/slang/slang-ast-modifier.h
Original file line number Diff line number Diff line change
Expand Up @@ -693,6 +693,28 @@ 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
};

class UncheckedGLSLBindingLayoutAttribute : public UncheckedGLSLLayoutAttribute
{
SLANG_AST_CLASS(UncheckedGLSLBindingLayoutAttribute)

SLANG_UNREFLECTED
};

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 +876,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
165 changes: 159 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 @@ -1434,6 +1450,98 @@ 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 (name == "binding" || 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.

Instead of using name comparisons, just dynamic cast uncheckedAttr:

if (auto bindingAttr = as<UncheckedGLSLBindingLayoutAttribute>(uncheckedAttr))
{
     ...
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done for "binding" and "offset" but "set" and other glsl layout modifier names that will be added in the future do(or will) not have explicit types. Binding and offset explicitly have them just to work around the edge case for atomic counters.

{
// 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 (name == "binding")
{
uncheckedAttr->args.add(nullptr);
}
else
{
uncheckedAttr->args.add(uncheckedAttr->args[0]);
uncheckedAttr->args[0] = nullptr;
SLANG_ASSERT(uncheckedAttr->args[1] != nullptr);
}

SLANG_ASSERT(uncheckedAttr->args.getCount() == 2);
}
else if (name == "offset")
{
attr = m_astBuilder->create<GLSLOffsetLayoutAttribute>();
}
else
{
getSink()->diagnose(uncheckedAttr->loc, 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 @@ -1459,6 +1567,21 @@ Modifier* SemanticsVisitor::checkModifier(
return checkedAttr;
}

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

if (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 @@ -1907,6 +2030,36 @@ 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 (auto glslBindingAttribute = syntaxNode->findModifier<GLSLBindingAttribute>())
{
if (glslOffsetAttribute->args.getCount() == 0)
{
glslOffsetAttribute->offset = getGLSLBindingOffsetTracker()->getNextBindingOffset(
glslBindingAttribute->binding);
}
else
{
fairywreath marked this conversation as resolved.
Show resolved Hide resolved
const auto constVal = checkConstantIntVal(glslOffsetAttribute->args[0]);
SLANG_ASSERT(constVal != nullptr);

glslOffsetAttribute->offset = uint64_t(constVal->getValue());
getGLSLBindingOffsetTracker()->setBindingOffset(
glslBindingAttribute->binding,
glslOffsetAttribute->offset);
}
}
else
{
getSink()->diagnose(
glslOffsetAttribute->loc,
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