Skip to content

Commit

Permalink
fix bug with offset not properly managed from zero'ed position
Browse files Browse the repository at this point in the history
add test for multiple offset atomic counters and verification

adjust tests for syntax; work arounding a bug found (will make issue/and-or fix)
  • Loading branch information
ArielG-NV committed Mar 11, 2024
1 parent e1ab20f commit 775a1a9
Show file tree
Hide file tree
Showing 4 changed files with 242 additions and 133 deletions.
15 changes: 8 additions & 7 deletions source/slang/glsl.meta.slang
Original file line number Diff line number Diff line change
Expand Up @@ -4578,13 +4578,13 @@ __spirv_version(1.3) [require(spirv)]

//// GLSL atomic

// this is actaully just a fancy storage buffer, and will be
// converted into one as such
// The following type internally is a Shader Storage Buffer
// as per GL_EXT_vulkan_glsl_relaxed
__magic_type(GLSLAtomicUintType)
__intrinsic_type($(kIROp_GLSLAtomicUintType))
public struct atomic_uint{};

// teir of float refers to atomic extension support of float1 or float2.
// tier of float refers to atomic extension support of float1 or float2.
// if we are inside a atomic_float function we will run the check for float1 teir
// types and operations to enable the according ext needed for these operations

Expand Down Expand Up @@ -4767,7 +4767,8 @@ struct TypeClassSubclass {
const char* classType;
const char *subclassType;
const char *suffix;
} atomics[] =
};
const TypeClassSubclass atomics[6] =
{
{
"uint", "I", "U", "",
Expand Down Expand Up @@ -4968,7 +4969,7 @@ __glsl_version(430) [require(glsl)]
}

__glsl_version(430) [require(glsl)]
[ForceInline] public uint atomicCounterDecrement_GLSL_wrapper(atomic_uint c)
[ForceInline] public uint atomicCounterDecrement_GLSL_helper(atomic_uint c)
{
__target_switch
{
Expand Down Expand Up @@ -5007,7 +5008,7 @@ __glsl_version(430) [require(glsl)]
{
case glsl:
{
atomicCounterDecrement_GLSL_wrapper(c);
atomicCounterDecrement_GLSL_helper(c);
return atomicCounter(c);
}
case spirv:
Expand Down Expand Up @@ -5177,4 +5178,4 @@ __glsl_version(430) [require(glsl)]
};
}
}
}
}
63 changes: 33 additions & 30 deletions source/slang/slang-parser.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -237,10 +237,15 @@ namespace Slang
}
const int64_t getNextBindingOffset(int binding)
{
const int64_t currentOffset = bindingToByteOffset.getOrAddValue(binding, 0);
int64_t currentOffset;
if (bindingToByteOffset.addIfNotExists(binding, 0))
currentOffset = 0;
else
currentOffset = bindingToByteOffset.getValue(binding) + sizeof(uint32_t);

bindingToByteOffset.set(
binding,
currentOffset+4
currentOffset + sizeof(uint32_t)
);
return currentOffset;
}
Expand Down Expand Up @@ -4389,35 +4394,33 @@ namespace Slang
{
// we intentionally have findModifier twice since GLSLOffsetLayoutAttribute is rare
// better to early leave
if (auto bindingMod = modifiers->findModifier<GLSLBindingAttribute>())
{
if (auto varDeclBase = as<VarDeclBase>(decl))
{
if (auto declRefExpr = as<DeclRefExpr>(varDeclBase->type.exp))
auto bindingMod = modifiers->findModifier<GLSLBindingAttribute>();
if (!bindingMod) return;
auto varDeclBase = as<VarDeclBase>(decl);
if (!varDeclBase) return;
auto declRefExpr = as<DeclRefExpr>(varDeclBase->type.exp);
if (!declRefExpr) return;

// here is a problem; we link types into a literal in IR stage post parse
// but, order (top down) mattter when parsing atomic_uint offset
// more over, we can have patterns like: offset = 20, no offset [+4], offset = 16.
// Therefore we must parse all in order. The issue then is we will struggle to
// subsitute atomic_uint for storage buffers...
if (auto name = declRefExpr->name)
{
if (name->text.equals("atomic_uint"))
{
if (!modifiers->findModifier<GLSLOffsetLayoutAttribute>())
{
// here is a problem; we link types into a literal in IR stage post parse
// but, order (top down) mattter when parsing atomic_uint offset
// more over, we can have patterns like: offset = 20, no offset [+4], offset = 16.
// Therefore we must parse all in order. The issue then is will struggle to
// subsitute atomic_uint for storage buffers...
if (auto name = declRefExpr->name)
{
if (name->text.equals("atomic_uint"))
{
if (!modifiers->findModifier<GLSLOffsetLayoutAttribute>())
{
const int64_t nextOffset = parser->getNextBindingOffset(bindingMod->binding);
GLSLOffsetLayoutAttribute* modifier = parser->astBuilder->create<GLSLOffsetLayoutAttribute>();
modifier->keywordName = NULL; //no keyword name given
modifier->loc = bindingMod->loc; //has no location in file, set to parent binding
modifier->offset = nextOffset;

Modifiers newModifier;
newModifier.first = modifier;
_addModifiers(decl, newModifier);
}
}
}
const int64_t nextOffset = parser->getNextBindingOffset(bindingMod->binding);
GLSLOffsetLayoutAttribute* modifier = parser->astBuilder->create<GLSLOffsetLayoutAttribute>();
modifier->keywordName = NULL; //no keyword name given
modifier->loc = bindingMod->loc; //has no location in file, set to parent binding
modifier->offset = nextOffset;

Modifiers newModifier;
newModifier.first = modifier;
_addModifiers(decl, newModifier);
}
}
}
Expand Down
33 changes: 33 additions & 0 deletions tests/glsl-intrinsic/atomic/atomicCounterTestMultiple.slang
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
//DIAGNOSTIC_TEST:SIMPLE(filecheck=CHECK_GLSL): -allow-glsl -stage compute -entry computeMain -target glsl -DTARGET_GLSL
#version 430

//TEST_INPUT:ubuffer(data=[0], stride=4):out,name=outputBuffer
buffer MyBlockName
{
uint data[1];
} outputBuffer;

layout(binding = 1, offset = 12) uniform atomic_uint one;
layout(binding = 1) uniform atomic_uint two;
layout(binding = 1, offset = 4) uniform atomic_uint three;
layout(binding = 1) uniform atomic_uint four;
layout(binding = 2) uniform atomic_uint five;

void computeMain()
{

outputBuffer.data[0] = true
// CHECK_GLSL-DAG: one_0._data_0[3]
&& atomicCounter(one) == 0
// CHECK_GLSL-DAG: one_0._data_0[4]
&& atomicCounter(two) == 0
// CHECK_GLSL-DAG: one_0._data_0[1]
&& atomicCounter(three) == 0
// CHECK_GLSL-DAG: one_0._data_0[2]
&& atomicCounter(four) == 0
// CHECK_GLSL-DAG: five_0._data_1[0]
&& atomicCounter(five) == 0

;

}
Loading

0 comments on commit 775a1a9

Please sign in to comment.