Fix Varying Variable Location Assignments With Hull Shaders #4915
Conversation
Fixes: shader-slang#4913 Fixes: shader-slang#4540 Changes: 1. Added `kIROp_ControlBarrier` to HLSL/GLSL emitting. 2. Added a method to track 'used' and 'unused' varyings for when legalizing GLSL. This allows us to assign correct offsets to automatically added varyings * Added a `ZeroLSB` check to UIntSet for this purpose
…s://github.com/ArielG-NV/slang into Varying-Location-Assignments-With-Hull-Shaders
source/slang/slang-emit-hlsl.cpp
Outdated
| @@ -500,6 +500,11 @@ bool HLSLSourceEmitter::tryEmitInstExprImpl(IRInst* inst, const EmitOpInfo& inOu | |||
| { | |||
| switch (inst->getOp()) | |||
| { | |||
| case kIROp_ControlBarrier: | |||
| { | |||
| m_writer->emit("GroupMemoryBarrier();\n"); | |||
There was a problem hiding this comment.
GroupMemoryBatrierWithGroupSync.
Otherwise this is just a memory barrier not a control barrier.
| Stage stage; | ||
| IRFunc* entryPointFunc; | ||
|
|
||
| //Dictionary<ResourceType, Dictionary<Space, BindingIndex>> |
There was a problem hiding this comment.
Add more description to the comment as it currently looks too much like a left over debug code.
| @@ -265,7 +265,8 @@ struct GLSLLegalizationContext | |||
| Stage stage; | |||
| IRFunc* entryPointFunc; | |||
|
|
|||
| //Dictionary<ResourceType, Dictionary<Space, BindingIndex>> | |||
| /// This dictionary stores for 'VaryingIn/VaryingOut' ('Dictionary<LayoutResourceKind, ...>') all used | |||
There was a problem hiding this comment.
Why would a varying in/out has a space binding? They should only have a single location number, so it appears to me that Dictionary<LayoutResourceKind, UIntSet> is sufficient.
There was a problem hiding this comment.
Why would a varying in/out has a space binding?
I am worried about some random crash or invalid code-gen due to a non '0' space for an edge case.
varying's should not have a space other than 0, although, I did not see anywhere that the GLSL legalization code to ensure any of this.
There was a problem hiding this comment.
We can safely assume this, because there is no way in spirv/glsl to specify a "space" for a varying inout.
There was a problem hiding this comment.
I will add this change then👍
Fixes: #4913
Fixes: #4540
Changes:
kIROp_ControlBarrierto HLSL/GLSL emitting.getLSBZerologic to UIntSet for efficient LSBZero finding logicNotes: