Skip to content

Commit

Permalink
Address glslang ordering requirments for 'derivative_group_*NV' (shad…
Browse files Browse the repository at this point in the history
…er-slang#4323)

* Address glslang ordering requirments for 'derivative_group_*NV'

fixes: shader-slang#4305

The solution is to emit some `layout`s after a module source is emitted.

Added to slangs gfx backend code to enable the compute shader derivative extension for testing purposes.

* address review

* enable removed test

---------

Co-authored-by: Yong He <yonghe@outlook.com>
  • Loading branch information
ArielG-NV and csyonghe committed Jun 10, 2024
1 parent 72016f9 commit 21bbebb
Show file tree
Hide file tree
Showing 8 changed files with 74 additions and 11 deletions.
7 changes: 5 additions & 2 deletions source/slang/slang-emit-c-like.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -129,9 +129,12 @@ void CLikeSourceEmitter::emitPreModuleImpl()
m_writer->emit(prelude->getStringSlice());
m_writer->emit("\n");
}
for (auto prelude : m_requiredPreludesRaw)
}
void CLikeSourceEmitter::emitPostModuleImpl()
{
if(m_requiredAfter.requireComputeDerivatives.getLength() > 0)
{
m_writer->emit(prelude);
m_writer->emit(m_requiredAfter.requireComputeDerivatives);
m_writer->emit("\n");
}
}
Expand Down
8 changes: 6 additions & 2 deletions source/slang/slang-emit-c-like.h
Original file line number Diff line number Diff line change
Expand Up @@ -431,7 +431,7 @@ class CLikeSourceEmitter: public SourceEmitterBase
void emitFrontMatter(TargetRequest* targetReq) { emitFrontMatterImpl(targetReq); }

void emitPreModule() { emitPreModuleImpl(); }

void emitPostModule() { emitPostModuleImpl(); }
void emitModule(IRModule* module, DiagnosticSink* sink)
{ m_irModule = module; emitModuleImpl(module, sink); }

Expand Down Expand Up @@ -476,6 +476,7 @@ class CLikeSourceEmitter: public SourceEmitterBase
/// For example on targets that don't have built in vector/matrix support, this is where
/// the appropriate generated declarations occur.
virtual void emitPreModuleImpl();
virtual void emitPostModuleImpl();

virtual void beforeComputeEmitActions(IRModule* module) { SLANG_UNUSED(module); };

Expand Down Expand Up @@ -590,8 +591,11 @@ class CLikeSourceEmitter: public SourceEmitterBase
// to use for it when emitting code.
Dictionary<IRInst*, String> m_mapInstToName;

OrderedHashSet<String> m_requiredPreludesRaw;
OrderedHashSet<IRStringLit*> m_requiredPreludes;
struct RequiredAfter
{
String requireComputeDerivatives;
}m_requiredAfter;
};

}
Expand Down
8 changes: 3 additions & 5 deletions source/slang/slang-emit-glsl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2208,9 +2208,7 @@ void GLSLSourceEmitter::handleRequiredCapabilitiesImpl(IRInst* inst)
{
// only allowed 1 of derivative_group_quadsNV or derivative_group_linearNV
if (m_entryPointStage != Stage::Compute
|| m_requiredPreludesRaw.contains("layout(derivative_group_quadsNV) in;")
|| m_requiredPreludesRaw.contains("layout(derivative_group_linearNV) in;")
)
|| m_requiredAfter.requireComputeDerivatives.getLength() > 0)
return;

_requireGLSLExtension(UnownedStringSlice("GL_NV_compute_shader_derivatives"));
Expand All @@ -2225,12 +2223,12 @@ void GLSLSourceEmitter::handleRequiredCapabilitiesImpl(IRInst* inst)
if (isQuad)
{
verifyComputeDerivativeGroupModifiers(getSink(), inst->sourceLoc, true, false, numThreadsDecor);
m_requiredPreludesRaw.add("layout(derivative_group_quadsNV) in;");
m_requiredAfter.requireComputeDerivatives = "layout(derivative_group_quadsNV) in;";
}
else
{
verifyComputeDerivativeGroupModifiers(getSink(), inst->sourceLoc, false, true, numThreadsDecor);
m_requiredPreludesRaw.add("layout(derivative_group_linearNV) in;");
m_requiredAfter.requireComputeDerivatives = "layout(derivative_group_linearNV) in;";
}
}
}
Expand Down
4 changes: 4 additions & 0 deletions source/slang/slang-emit.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1587,6 +1587,10 @@ SlangResult CodeGenContext::emitEntryPointsSourceFromIR(ComPtr<IArtifact>& outAr
// Append the modules output code
finalResult.append(code);

// Append all content that should be at the end of a module
sourceEmitter->emitPostModule();
finalResult.append(sourceWriter.getContentAndClear());

// Write out the result

auto artifact = ArtifactUtil::createArtifactForCompileTarget(asExternal(target));
Expand Down
40 changes: 40 additions & 0 deletions tests/bugs/gh-4305.slang
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
//TEST:SIMPLE(filecheck=CHECK): -stage compute -entry computeMain -target glsl -allow-glsl -DQUAD -DMODIFIER
//TEST:SIMPLE(filecheck=CHECK): -stage compute -entry computeMain -target glsl -allow-glsl -DMODIFIER
//TEST:SIMPLE(filecheck=CHECK): -stage compute -entry computeMain -target glsl -allow-glsl -DQUAD
//TEST:SIMPLE(filecheck=CHECK): -stage compute -entry computeMain -target glsl -allow-glsl
//TEST(compute, vulkan):COMPARE_COMPUTE(filecheck-buffer=BUF):-vk -compute -entry computeMain -output-using-type -emit-spirv-via-glsl -allow-glsl -xslang -DQUAD
//TEST(compute, vulkan):COMPARE_COMPUTE(filecheck-buffer=BUF):-vk -compute -entry computeMain -output-using-type -emit-spirv-via-glsl -allow-glsl

//TEST_INPUT: ubuffer(data=[0], stride=4):out,name outputBuffer
RWStructuredBuffer<float> outputBuffer;
//TEST_INPUT: Texture2D(size=4, content = one):name t2D
Texture2D<float> t2D;
//TEST_INPUT: Sampler:name samplerState
SamplerState samplerState;

// "local_size_x" must appear before "derivative_group_"
//CHECK:local_size_x
//CHECK:derivative_group_{{.*}}NV

#ifndef MODIFIER
#ifdef QUAD
layout(derivative_group_quadsNV) in;
#else
layout(derivative_group_linearNV) in;
#endif // #ifdef QUAD
layout(local_size_x = 2, local_size_y = 2, local_size_z = 1) in;

#else

#ifdef QUAD
[DerivativeGroupQuad]
#else
[DerivativeGroupLinear]
#endif // #ifdef QUAD
[numthreads(2, 2, 1)]
#endif //#ifndef MODIFIER
void computeMain()
{
//BUF:1
outputBuffer[0] = t2D.Sample(samplerState, float2(0.5, 0.5));
}
3 changes: 1 addition & 2 deletions tests/glsl-intrinsic/intrinsic-texture.slang
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,7 @@
//TEST:SIMPLE(filecheck=CUDA): -allow-glsl -stage compute -entry computeMain -target cuda
//TEST:SIMPLE(filecheck=CUDA): -allow-glsl -stage fragment -entry fragMain -target cuda

// Enable this test when an issue#4305 is addressed.
//T-EST(compute, vulkan):COMPARE_COMPUTE(filecheck-buffer=BUF):-vk -compute -entry computeMain -allow-glsl -output-using-type
//TEST(compute, vulkan):COMPARE_COMPUTE(filecheck-buffer=BUF):-vk -compute -entry computeMain -allow-glsl -output-using-type


//TEST_INPUT:ubuffer(data=[0], stride=4):out,name=outputBuffer
Expand Down
4 changes: 4 additions & 0 deletions tools/gfx/vulkan/vk-api.h
Original file line number Diff line number Diff line change
Expand Up @@ -287,6 +287,10 @@ struct VulkanExtendedFeatureProperties
VK_STRUCTURE_TYPE_PHYSICAL_DEVICE_VARIABLE_POINTER_FEATURES_KHR
};

VkPhysicalDeviceComputeShaderDerivativesFeaturesNV computeShaderDerivativeFeatures = {
VK_STRUCTURE_TYPE_PHYSICAL_DEVICE_COMPUTE_SHADER_DERIVATIVES_FEATURES_NV
};

// Clock features
VkPhysicalDeviceShaderClockFeaturesKHR clockFeatures = {
VK_STRUCTURE_TYPE_PHYSICAL_DEVICE_SHADER_CLOCK_FEATURES_KHR
Expand Down
11 changes: 11 additions & 0 deletions tools/gfx/vulkan/vk-device.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -464,6 +464,10 @@ Result DeviceImpl::initVulkanInstanceAndDevice(
extendedFeatures.variablePointersFeatures.pNext = deviceFeatures2.pNext;
deviceFeatures2.pNext = &extendedFeatures.variablePointersFeatures;

// Compute shader derivative features.
extendedFeatures.computeShaderDerivativeFeatures.pNext = deviceFeatures2.pNext;
deviceFeatures2.pNext = &extendedFeatures.computeShaderDerivativeFeatures;

// Extended dynamic states
extendedFeatures.extendedDynamicStateFeatures.pNext = deviceFeatures2.pNext;
deviceFeatures2.pNext = &extendedFeatures.extendedDynamicStateFeatures;
Expand Down Expand Up @@ -696,6 +700,13 @@ Result DeviceImpl::initVulkanInstanceAndDevice(
VK_KHR_VARIABLE_POINTERS_EXTENSION_NAME,
"variable-pointer"
);

SIMPLE_EXTENSION_FEATURE(
extendedFeatures.computeShaderDerivativeFeatures,
computeDerivativeGroupLinear,
VK_NV_COMPUTE_SHADER_DERIVATIVES_EXTENSION_NAME,
"computeDerivativeGroupLinear"
);

#undef SIMPLE_EXTENSION_FEATURE

Expand Down

0 comments on commit 21bbebb

Please sign in to comment.