Merge pull request #1230 from KhronosGroup/cleanup-workgroup-barrier

HansKristian-Work · web-flow · commit 15b860eb1c95 · 2019-12-04T16:41:34.000+01:00
Don't emit memoryBarrierShared() in workgroup control barriers.
diff --git a/reference/opt/shaders/comp/barriers.comp b/reference/opt/shaders/comp/barriers.comp
@@ -8,21 +8,15 @@ void main()
     memoryBarrierImage();
     memoryBarrierBuffer();
     groupMemoryBarrier();
-    memoryBarrierShared();
     barrier();
     memoryBarrier();
-    memoryBarrierShared();
     barrier();
     memoryBarrierImage();
-    memoryBarrierShared();
     barrier();
     memoryBarrierBuffer();
-    memoryBarrierShared();
     barrier();
     groupMemoryBarrier();
-    memoryBarrierShared();
     barrier();
-    memoryBarrierShared();
     barrier();
 }
 
diff --git a/reference/opt/shaders/comp/shared.comp b/reference/opt/shaders/comp/shared.comp
@@ -16,7 +16,6 @@ shared float sShared[4];
 void main()
 {
     sShared[gl_LocalInvocationIndex] = _22.in_data[gl_GlobalInvocationID.x];
-    memoryBarrierShared();
     barrier();
     _44.out_data[gl_GlobalInvocationID.x] = sShared[3u - gl_LocalInvocationIndex];
 }
diff --git a/reference/shaders/comp/barriers.comp b/reference/shaders/comp/barriers.comp
@@ -28,41 +28,35 @@ void group_barrier()
 
 void barrier_shared_exec()
 {
-    memoryBarrierShared();
     barrier();
 }
 
 void full_barrier_exec()
 {
     memoryBarrier();
-    memoryBarrierShared();
     barrier();
 }
 
 void image_barrier_exec()
 {
     memoryBarrierImage();
-    memoryBarrierShared();
     barrier();
 }
 
 void buffer_barrier_exec()
 {
     memoryBarrierBuffer();
-    memoryBarrierShared();
     barrier();
 }
 
 void group_barrier_exec()
 {
     groupMemoryBarrier();
-    memoryBarrierShared();
     barrier();
 }
 
 void exec_barrier()
 {
-    memoryBarrierShared();
     barrier();
 }
 
diff --git a/reference/shaders/comp/shared.comp b/reference/shaders/comp/shared.comp
@@ -18,7 +18,6 @@ void main()
     uint ident = gl_GlobalInvocationID.x;
     float idata = _22.in_data[ident];
     sShared[gl_LocalInvocationIndex] = idata;
-    memoryBarrierShared();
     barrier();
     _44.out_data[ident] = sShared[(4u - gl_LocalInvocationIndex) - 1u];
 }
diff --git a/spirv_glsl.cpp b/spirv_glsl.cpp
@@ -9892,7 +9892,12 @@ void CompilerGLSL::emit_instruction(const Instruction &instruction)
 		if (memory == ScopeWorkgroup) // Only need to consider memory within a group
 		{
 			if (semantics == MemorySemanticsWorkgroupMemoryMask)
-				statement("memoryBarrierShared();");
+			{
+				// OpControlBarrier implies a memory barrier for shared memory as well.
+				bool implies_shared_barrier = opcode == OpControlBarrier && execution_scope == ScopeWorkgroup;
+				if (!implies_shared_barrier)
+					statement("memoryBarrierShared();");
+			}
 			else if (semantics != 0)
 				statement("groupMemoryBarrier();");
 		}
@@ -9926,7 +9931,7 @@ void CompilerGLSL::emit_instruction(const Instruction &instruction)
 		else
 		{
 			const uint32_t all_barriers = MemorySemanticsWorkgroupMemoryMask | MemorySemanticsUniformMemoryMask |
-			                              MemorySemanticsImageMemoryMask | MemorySemanticsAtomicCounterMemoryMask;
+			                              MemorySemanticsImageMemoryMask;
 
 			if (semantics & (MemorySemanticsCrossWorkgroupMemoryMask | MemorySemanticsSubgroupMemoryMask))
 			{
@@ -9948,8 +9953,6 @@ void CompilerGLSL::emit_instruction(const Instruction &instruction)
 					statement("memoryBarrierBuffer();");
 				if (semantics & MemorySemanticsImageMemoryMask)
 					statement("memoryBarrierImage();");
-				if (semantics & MemorySemanticsAtomicCounterMemoryMask)
-					statement("memoryBarrierAtomicCounter();");
 			}
 		}
 
diff --git a/spirv_msl.cpp b/spirv_msl.cpp
@@ -6513,8 +6513,7 @@ void CompilerMSL::emit_barrier(uint32_t id_exe_scope, uint32_t id_mem_scope, uin
 
 		// Fix tessellation patch function processing
 		if (get_execution_model() == ExecutionModelTessellationControl ||
-		    (mem_sem & (MemorySemanticsSubgroupMemoryMask | MemorySemanticsWorkgroupMemoryMask |
-		                MemorySemanticsAtomicCounterMemoryMask)))
+		    (mem_sem & (MemorySemanticsSubgroupMemoryMask | MemorySemanticsWorkgroupMemoryMask)))
 		{
 			if (!mem_flags.empty())
 				mem_flags += " | ";
@@ -6535,13 +6534,11 @@ void CompilerMSL::emit_barrier(uint32_t id_exe_scope, uint32_t id_mem_scope, uin
 	else
 	{
 		if ((mem_sem & (MemorySemanticsUniformMemoryMask | MemorySemanticsCrossWorkgroupMemoryMask)) &&
-		    (mem_sem & (MemorySemanticsSubgroupMemoryMask | MemorySemanticsWorkgroupMemoryMask |
-		                MemorySemanticsAtomicCounterMemoryMask)))
+		    (mem_sem & (MemorySemanticsSubgroupMemoryMask | MemorySemanticsWorkgroupMemoryMask)))
 			bar_stmt += "mem_flags::mem_device_and_threadgroup";
 		else if (mem_sem & (MemorySemanticsUniformMemoryMask | MemorySemanticsCrossWorkgroupMemoryMask))
 			bar_stmt += "mem_flags::mem_device";
-		else if (mem_sem & (MemorySemanticsSubgroupMemoryMask | MemorySemanticsWorkgroupMemoryMask |
-		                    MemorySemanticsAtomicCounterMemoryMask))
+		else if (mem_sem & (MemorySemanticsSubgroupMemoryMask | MemorySemanticsWorkgroupMemoryMask))
 			bar_stmt += "mem_flags::mem_threadgroup";
 		else if (mem_sem & MemorySemanticsImageMemoryMask)
 			bar_stmt += "mem_flags::mem_texture";

Original file line number	Diff line number	Diff line change
`@@ -16,7 +16,6 @@ shared float sShared[4];`
`16`	`16`	`void main()`
`17`	`17`	`{`
`18`	`18`	`sShared[gl_LocalInvocationIndex] = _22.in_data[gl_GlobalInvocationID.x];`
`19`		`- memoryBarrierShared();`
`20`	`19`	`barrier();`
`21`	`20`	`_44.out_data[gl_GlobalInvocationID.x] = sShared[3u - gl_LocalInvocationIndex];`
`22`	`21`	`}`
Original file line number	Diff line number	Diff line change
`@@ -28,41 +28,35 @@ void group_barrier()`
`28`	`28`
`29`	`29`	`void barrier_shared_exec()`
`30`	`30`	`{`
`31`		`- memoryBarrierShared();`
`32`	`31`	`barrier();`
`33`	`32`	`}`
`34`	`33`
`35`	`34`	`void full_barrier_exec()`
`36`	`35`	`{`
`37`	`36`	`memoryBarrier();`
`38`		`- memoryBarrierShared();`
`39`	`37`	`barrier();`
`40`	`38`	`}`
`41`	`39`
`42`	`40`	`void image_barrier_exec()`
`43`	`41`	`{`
`44`	`42`	`memoryBarrierImage();`
`45`		`- memoryBarrierShared();`
`46`	`43`	`barrier();`
`47`	`44`	`}`
`48`	`45`
`49`	`46`	`void buffer_barrier_exec()`
`50`	`47`	`{`
`51`	`48`	`memoryBarrierBuffer();`
`52`		`- memoryBarrierShared();`
`53`	`49`	`barrier();`
`54`	`50`	`}`
`55`	`51`
`56`	`52`	`void group_barrier_exec()`
`57`	`53`	`{`
`58`	`54`	`groupMemoryBarrier();`
`59`		`- memoryBarrierShared();`
`60`	`55`	`barrier();`
`61`	`56`	`}`
`62`	`57`
`63`	`58`	`void exec_barrier()`
`64`	`59`	`{`
`65`		`- memoryBarrierShared();`
`66`	`60`	`barrier();`
`67`	`61`	`}`
`68`	`62`
Original file line number	Diff line number	Diff line change
`@@ -18,7 +18,6 @@ void main()`
`18`	`18`	`uint ident = gl_GlobalInvocationID.x;`
`19`	`19`	`float idata = _22.in_data[ident];`
`20`	`20`	`sShared[gl_LocalInvocationIndex] = idata;`
`21`		`- memoryBarrierShared();`
`22`	`21`	`barrier();`
`23`	`22`	`_44.out_data[ident] = sShared[(4u - gl_LocalInvocationIndex) - 1u];`
`24`	`23`	`}`
Original file line number	Diff line number	Diff line change
`@@ -9892,7 +9892,12 @@ void CompilerGLSL::emit_instruction(const Instruction &instruction)`
`9892`	`9892`	`if (memory == ScopeWorkgroup) // Only need to consider memory within a group`
`9893`	`9893`	`{`
`9894`	`9894`	`if (semantics == MemorySemanticsWorkgroupMemoryMask)`
`9895`		`- statement("memoryBarrierShared();");`
	`9895`	`+ {`
	`9896`	`+ // OpControlBarrier implies a memory barrier for shared memory as well.`
	`9897`	`+ bool implies_shared_barrier = opcode == OpControlBarrier && execution_scope == ScopeWorkgroup;`
	`9898`	`+ if (!implies_shared_barrier)`
	`9899`	`+ statement("memoryBarrierShared();");`
	`9900`	`+ }`
`9896`	`9901`	`else if (semantics != 0)`
`9897`	`9902`	`statement("groupMemoryBarrier();");`
`9898`	`9903`	`}`
`@@ -9926,7 +9931,7 @@ void CompilerGLSL::emit_instruction(const Instruction &instruction)`
`9926`	`9931`	`else`
`9927`	`9932`	`{`
`9928`	`9933`	`const uint32_t all_barriers = MemorySemanticsWorkgroupMemoryMask \| MemorySemanticsUniformMemoryMask \|`
`9929`		`- MemorySemanticsImageMemoryMask \| MemorySemanticsAtomicCounterMemoryMask;`
	`9934`	`+ MemorySemanticsImageMemoryMask;`
`9930`	`9935`
`9931`	`9936`	`if (semantics & (MemorySemanticsCrossWorkgroupMemoryMask \| MemorySemanticsSubgroupMemoryMask))`
`9932`	`9937`	`{`
`@@ -9948,8 +9953,6 @@ void CompilerGLSL::emit_instruction(const Instruction &instruction)`
`9948`	`9953`	`statement("memoryBarrierBuffer();");`
`9949`	`9954`	`if (semantics & MemorySemanticsImageMemoryMask)`
`9950`	`9955`	`statement("memoryBarrierImage();");`
`9951`		`- if (semantics & MemorySemanticsAtomicCounterMemoryMask)`
`9952`		`- statement("memoryBarrierAtomicCounter();");`
`9953`	`9956`	`}`
`9954`	`9957`	`}`
`9955`	`9958`