Merge pull request #357 from KhronosGroup/pr-353

Fixups for PR #353

.gitignore

@@ -14,5 +14,6 @@
 *.shader
 *.a
 *.bc
+/external
 
 !CMakeLists.txt

README.md

@@ -264,12 +264,36 @@ Contributions to SPIRV-Cross are welcome. See Testing and Licensing sections for
 SPIRV-Cross maintains a test suite of shaders with reference output of how the output looks after going through a roundtrip through
 glslangValidator then back through SPIRV-Cross again. The reference files are stored inside the repository in order to be able to track regressions.
 
-All pull requests should ensure that test output does not change unexpectedly. This can be tested with `./test_shaders.py shaders`.
-However, when improving SPIRV-Cross there are of course legitimate cases where reference output should change.
-In these cases, run `./test_shaders.py shaders --update` to update the reference files and include these changes as part of the pull request.
-Always make sure you are running up to date glslangValidator as well as SPIRV-Tools when updating reference files.
+All pull requests should ensure that test output does not change unexpectedly. This can be tested with:
 
-In short, the master branch should always be able to run `./test_shaders.py shaders` without failure.
+```
+./test_shaders.py shaders
+./test_shaders.py shaders --opt
+./test_shaders.py shaders-hlsl --hlsl
+./test_shaders.py shaders-hlsl --hlsl --opt
+./test_shaders.py shaders-msl --msl
+./test_shaders.py shaders-msl --msl --opt
+```
+
+although there are a couple of convenience script for doing this:
+
+```
+./checkout_glslang_spirv_tools.sh # Checks out glslang and SPIRV-Tools at a fixed revision which matches the reference output.
+./test_shaders.sh                 # Runs over all changes and makes sure that there are no deltas compared to reference files.
+```
+
+However, when improving SPIRV-Cross there are of course legitimate cases where reference output should change.
+In these cases, run:
+
+```
+./update_test_shaders.sh
+```
+
+to update the reference files and include these changes as part of the pull request.
+Always make sure you are running the correct version of glslangValidator as well as SPIRV-Tools when updating reference files.
+See `checkout_glslang_spirv_tools.sh`.
+
+In short, the master branch should always be able to run `./test_shaders.py shaders` and friends without failure.
 SPIRV-Cross uses Travis CI to test all pull requests, so it is not strictly needed to perform testing yourself if you have problems running it locally.
 A pull request which does not pass testing on Travis will not be accepted however.
 

checkout_glslang_spirv_tools.sh

@@ -0,0 +1,57 @@
+#!/bin/bash
+
+GLSLANG_REV=698bf7547a96b6feb7291e8ddc0d5d16475dbae2
+SPIRV_TOOLS_REV=e28edd458b729da7bbfd51e375feb33103709e6f
+
+if [ -d external/glslang ]; then
+	echo "Updating glslang to revision $GLSLANG_REV."
+	cd external/glslang
+	git fetch origin
+	git checkout $GLSLANG_REV
+else
+	echo "Cloning glslang revision $GLSLANG_REV."
+	mkdir -p external
+	cd external
+	git clone git://github.com/KhronosGroup/glslang.git
+	cd glslang
+	git checkout $GLSLANG_REV
+fi
+cd ../..
+
+echo "Building glslang."
+mkdir -p external/glslang-build
+cd external/glslang-build
+cmake ../glslang -DCMAKE_BUILD_TYPE=Release
+make -j$(nproc)
+cd ../..
+
+if [ -d external/spirv-tools ]; then
+	echo "Updating SPIRV-Tools to revision $SPIRV_TOOLS_REV."
+	cd external/spirv-tools
+	git fetch origin
+	git checkout $SPIRV_TOOLS_REV
+else
+	echo "Cloning SPIRV-Tools revision $SPIRV_TOOLS_REV."
+	mkdir -p external
+	cd external
+	git clone git://github.com/KhronosGroup/SPIRV-Tools.git spirv-tools
+	cd spirv-tools
+	git checkout $SPIRV_TOOLS_REV
+
+	if [ -d external/spirv-headers ]; then
+		cd external/spirv-headers
+		git pull origin master
+		cd ../..
+	else
+		git clone git://github.com/KhronosGroup/SPIRV-Headers.git external/spirv-headers
+	fi
+fi
+cd ../..
+
+echo "Building SPIRV-Tools."
+mkdir -p external/spirv-tools-build
+cd external/spirv-tools-build
+cmake ../spirv-tools -DCMAKE_BUILD_TYPE=Release
+make -j$(nproc)
+cd ../..
+

reference/opt/shaders-msl/asm/frag/default-member-names.asm.frag

@@ -25,7 +25,7 @@ struct _10
     _9 _m12;
 };
 
-constant _10 _51 = _10();
+constant _10 _51 = {};
 
 struct main0_out
 {

reference/opt/shaders-msl/asm/frag/undef-variable-store.asm.frag

@@ -3,8 +3,8 @@
 
 using namespace metal;
 
-constant float4 _38 = float4(0);
-constant float4 _50 = float4(0);
+constant float4 _38 = {};
+constant float4 _50 = {};
 
 struct main0_out
 {

reference/opt/shaders-msl/comp/insert.comp

@@ -8,7 +8,7 @@ struct SSBO
     float4 out_data[1];
 };
 
-constant float4 _52 = float4(0);
+constant float4 _52 = {};
 
 kernel void main0(device SSBO& _27 [[buffer(0)]], uint3 gl_GlobalInvocationID [[thread_position_in_grid]])
 {

reference/opt/shaders-msl/comp/return.comp

@@ -8,7 +8,7 @@ struct SSBO2
     float4 out_data[1];
 };
 
-constant int _69 = int(0);
+constant int _69 = {};
 
 kernel void main0(uint3 gl_GlobalInvocationID [[thread_position_in_grid]], device SSBO2& _27 [[buffer(0)]])
 {

reference/opt/shaders-msl/comp/torture-loop.comp

@@ -14,7 +14,7 @@ struct SSBO2
     float4 out_data[1];
 };
 
-constant uint _98 = uint(0);
+constant uint _98 = {};
 
 kernel void main0(uint3 gl_GlobalInvocationID [[thread_position_in_grid]], device SSBO& _24 [[buffer(0)]], device SSBO2& _89 [[buffer(1)]])
 {

reference/shaders-msl/asm/frag/undef-variable-store.asm.frag

@@ -3,8 +3,8 @@
 
 using namespace metal;
 
-constant float4 _38 = float4(0);
-constant float4 _47 = float4(0);
+constant float4 _38 = {};
+constant float4 _47 = {};
 
 struct main0_out
 {

shaders-msl/comp/bake_gradient.comp

@@ -1,55 +0,0 @@
-#version 310 es
-
-layout(local_size_x = 8, local_size_y = 8) in;
-
-layout(binding = 0) uniform sampler2D uHeight;
-layout(binding = 1) uniform sampler2D uDisplacement;
-layout(rgba16f, binding = 2) uniform writeonly mediump image2D iHeightDisplacement;
-layout(rgba16f, binding = 3) uniform writeonly mediump image2D iGradJacobian;
-
-layout(binding = 4) uniform UBO
-{
-    vec4 uInvSize;
-    vec4 uScale;
-};
-
-mediump float jacobian(mediump vec2 dDdx, mediump vec2 dDdy)
-{
-    return (1.0 + dDdx.x) * (1.0 + dDdy.y) - dDdx.y * dDdy.x;
-}
-#define LAMBDA 1.2
-
-void main()
-{
-    vec4 uv = (vec2(gl_GlobalInvocationID.xy) * uInvSize.xy).xyxy + 0.5 * uInvSize;
-
-    float h = textureLod(uHeight, uv.xy, 0.0).x;
-
-    // Compute the heightmap gradient by simple differentiation.
-    float x0 = textureLodOffset(uHeight, uv.xy, 0.0, ivec2(-1, 0)).x;
-    float x1 = textureLodOffset(uHeight, uv.xy, 0.0, ivec2(+1, 0)).x;
-    float y0 = textureLodOffset(uHeight, uv.xy, 0.0, ivec2(0, -1)).x;
-    float y1 = textureLodOffset(uHeight, uv.xy, 0.0, ivec2(0, +1)).x;
-    vec2 grad = uScale.xy * 0.5 * vec2(x1 - x0, y1 - y0);
-
-    // Displacement map must be sampled with a different offset since it's a smaller texture.
-    vec2 displacement = LAMBDA * textureLod(uDisplacement, uv.zw, 0.0).xy;
-
-    // Compute jacobian.
-    vec2 dDdx = 0.5 * LAMBDA * (
-        textureLodOffset(uDisplacement, uv.zw, 0.0, ivec2(+1, 0)).xy -
-        textureLodOffset(uDisplacement, uv.zw, 0.0, ivec2(-1, 0)).xy);
-    vec2 dDdy = 0.5 * LAMBDA * (
-        textureLodOffset(uDisplacement, uv.zw, 0.0, ivec2(0, +1)).xy -
-        textureLodOffset(uDisplacement, uv.zw, 0.0, ivec2(0, -1)).xy);
-    float j = jacobian(dDdx * uScale.z, dDdy * uScale.z);
-
-    displacement = vec2(0.0);
-
-    // Read by vertex shader/tess shader.
-    imageStore(iHeightDisplacement, ivec2(gl_GlobalInvocationID.xy), vec4(h, displacement, 0.0));
-
-    // Read by fragment shader.
-    imageStore(iGradJacobian, ivec2(gl_GlobalInvocationID.xy), vec4(grad, j, 0.0));
-}
-

spirv_cross.cpp

@@ -679,7 +679,8 @@ ShaderResources Compiler::get_shader_resources(const unordered_set<uint32_t> *ac
 		if (var.storage == StorageClassInput && interface_variable_exists_in_entry_point(var.self))
 		{
 			if (meta[type.self].decoration.decoration_flags & (1ull << DecorationBlock))
-				res.stage_inputs.push_back({ var.self, var.basetype, type.self, get_remapped_declared_block_name(var.self) });
+				res.stage_inputs.push_back(
+				    { var.self, var.basetype, type.self, get_remapped_declared_block_name(var.self) });
 			else
 				res.stage_inputs.push_back({ var.self, var.basetype, type.self, meta[var.self].decoration.alias });
 		}
@@ -692,7 +693,8 @@ ShaderResources Compiler::get_shader_resources(const unordered_set<uint32_t> *ac
 		else if (var.storage == StorageClassOutput && interface_variable_exists_in_entry_point(var.self))
 		{
 			if (meta[type.self].decoration.decoration_flags & (1ull << DecorationBlock))
-				res.stage_outputs.push_back({ var.self, var.basetype, type.self, get_remapped_declared_block_name(var.self) });
+				res.stage_outputs.push_back(
+				    { var.self, var.basetype, type.self, get_remapped_declared_block_name(var.self) });
 			else
 				res.stage_outputs.push_back({ var.self, var.basetype, type.self, meta[var.self].decoration.alias });
 		}
@@ -700,21 +702,21 @@ ShaderResources Compiler::get_shader_resources(const unordered_set<uint32_t> *ac
 		else if (type.storage == StorageClassUniform &&
 		         (meta[type.self].decoration.decoration_flags & (1ull << DecorationBlock)))
 		{
-			res.uniform_buffers.push_back({ var.self, var.basetype, type.self,
-			                                get_remapped_declared_block_name(var.self) });
+			res.uniform_buffers.push_back(
+			    { var.self, var.basetype, type.self, get_remapped_declared_block_name(var.self) });
 		}
 		// Old way to declare SSBOs.
 		else if (type.storage == StorageClassUniform &&
 		         (meta[type.self].decoration.decoration_flags & (1ull << DecorationBufferBlock)))
 		{
-			res.storage_buffers.push_back({ var.self, var.basetype, type.self,
-			                                get_remapped_declared_block_name(var.self) });
+			res.storage_buffers.push_back(
+			    { var.self, var.basetype, type.self, get_remapped_declared_block_name(var.self) });
 		}
 		// Modern way to declare SSBOs.
 		else if (type.storage == StorageClassStorageBuffer)
 		{
-			res.storage_buffers.push_back({ var.self, var.basetype, type.self,
-			                                get_remapped_declared_block_name(var.self) });
+			res.storage_buffers.push_back(
+			    { var.self, var.basetype, type.self, get_remapped_declared_block_name(var.self) });
 		}
 		// Push constant blocks
 		else if (type.storage == StorageClassPushConstant)

spirv_msl.cpp

@@ -1042,7 +1042,7 @@ void CompilerMSL::emit_custom_functions()
 }
 
 // Undefined global memory is not allowed in MSL.
-// Declare constant and init to zeros.
+// Declare constant and init to zeros. Use {}, as global constructors can break Metal.
 void CompilerMSL::declare_undefined_values()
 {
 	bool emitted = false;
@@ -1052,21 +1052,7 @@ void CompilerMSL::declare_undefined_values()
 		{
 			auto &undef = id.get<SPIRUndef>();
 			auto &type = get<SPIRType>(undef.basetype);
-
-			string arg_str;
-			switch (type.basetype)
-			{
-			case SPIRType::Struct:
-				arg_str = "";
-				break;
-
-			default:
-				arg_str = "0";
-				break;
-			}
-			string init_str = type_to_glsl(type) + "(" + arg_str + ")";
-
-			statement("constant ", variable_decl(type, to_name(undef.self), undef.self), " = ", init_str, ";");
+			statement("constant ", variable_decl(type, to_name(undef.self), undef.self), " = {};");
 			emitted = true;
 		}
 	}

test_shaders.sh

@@ -0,0 +1,15 @@
+#!/bin/bash
+
+echo "Building spirv-cross"
+make -j$(nproc)
+
+export PATH="./external/glslang-build/StandAlone:./external/spirv-tools-build/tools:$PATH"
+echo "Using glslangValidation in: $(which glslangValidator)."
+echo "Using spirv-opt in: $(which spirv-opt)."
+
+./test_shaders.py shaders
+./test_shaders.py shaders --opt
+./test_shaders.py shaders-msl --msl
+./test_shaders.py shaders-msl --msl --opt
+./test_shaders.py shaders-hlsl --hlsl
+./test_shaders.py shaders-hlsl --hlsl --opt

update_test_shaders.sh

@@ -0,0 +1,15 @@
+#!/bin/bash
+
+echo "Building spirv-cross"
+make -j$(nproc)
+
+export PATH="./external/glslang-build/StandAlone:./external/spirv-tools-build/tools:$PATH"
+echo "Using glslangValidation in: $(which glslangValidator)."
+echo "Using spirv-opt in: $(which spirv-opt)."
+
+./test_shaders.py shaders --update
+./test_shaders.py shaders --update --opt
+./test_shaders.py shaders-msl --msl --update
+./test_shaders.py shaders-msl --msl --update --opt
+./test_shaders.py shaders-hlsl --hlsl --update
+./test_shaders.py shaders-hlsl --hlsl --update --opt