From f45075b08b51ab1caf1d6ec621c197dc13cf6a79 Mon Sep 17 00:00:00 2001 From: Arseny Kapoulkine Date: Tue, 24 Jan 2017 22:26:39 -0800 Subject: [PATCH 1/2] Validate Metal shaders on OSX with Metal compiler If we run on a system with Xcode installed to a default path, run Xcode Metal shader compiler to validate the generated MSL shader. This uncovers an issue in the existing MSL test - MSL backend currently does not auto-assign attribute locations, which means that translating GLSL shader without location layout produces an invalid MSL which generates "error: 'attribute' index '0' is used more than once". --- reference/shaders-msl/vert/basic.vert | 2 +- shaders-msl/vert/basic.vert | 6 ++++-- test_shaders.py | 19 +++++++++++++------ 3 files changed, 18 insertions(+), 9 deletions(-) diff --git a/reference/shaders-msl/vert/basic.vert b/reference/shaders-msl/vert/basic.vert index b895e104..d2c96d2c 100644 --- a/reference/shaders-msl/vert/basic.vert +++ b/reference/shaders-msl/vert/basic.vert @@ -10,8 +10,8 @@ struct UBO struct main0_in { + float3 aNormal [[attribute(1)]]; float4 aVertex [[attribute(0)]]; - float3 aNormal [[attribute(0)]]; }; struct main0_out diff --git a/shaders-msl/vert/basic.vert b/shaders-msl/vert/basic.vert index 801724f3..6d8bdc17 100644 --- a/shaders-msl/vert/basic.vert +++ b/shaders-msl/vert/basic.vert @@ -4,8 +4,10 @@ layout(std140) uniform UBO { uniform mat4 uMVP; }; -in vec4 aVertex; -in vec3 aNormal; + +layout(location = 0) in vec4 aVertex; +layout(location = 1) in vec3 aNormal; + out vec3 vNormal; void main() diff --git a/test_shaders.py b/test_shaders.py index 97d09bdf..a3004f41 100755 --- a/test_shaders.py +++ b/test_shaders.py @@ -2,6 +2,7 @@ import sys import os +import os.path import subprocess import tempfile import re @@ -60,11 +61,10 @@ def get_shader_stats(shader): returned = stdout.decode('utf-8') return parse_stats(returned) -def validate_shader(shader, vulkan): - if vulkan: - subprocess.check_call(['glslangValidator', '-V', shader]) - else: - subprocess.check_call(['glslangValidator', shader]) +def validate_shader_msl(shader): + path = "/Applications/Xcode.app/Contents/Developer/Platforms/iPhoneOS.platform/usr/bin/metal" + if os.path.exists(path): + subprocess.check_call([path, '-x', 'metal', '-std=ios-metal1.0', '-Werror', shader]) def cross_compile_msl(shader): spirv_f, spirv_path = tempfile.mkstemp() @@ -76,9 +76,16 @@ def cross_compile_msl(shader): subprocess.check_call([spirv_cross_path, '--entry', 'main', '--output', msl_path, spirv_path, '--metal']) subprocess.check_call(['spirv-val', spirv_path]) - # TODO: Add optional validation of the MSL output. + validate_shader_msl(msl_path) + return (spirv_path, msl_path) +def validate_shader(shader, vulkan): + if vulkan: + subprocess.check_call(['glslangValidator', '-V', shader]) + else: + subprocess.check_call(['glslangValidator', shader]) + def cross_compile(shader, vulkan, spirv, invalid_spirv, eliminate, is_legacy, flatten_ubo): spirv_f, spirv_path = tempfile.mkstemp() glsl_f, glsl_path = tempfile.mkstemp(suffix = os.path.basename(shader)) From 49baf419909a87cf8b762e747da30d48ae3d70b8 Mon Sep 17 00:00:00 2001 From: Arseny Kapoulkine Date: Wed, 25 Jan 2017 00:01:53 -0800 Subject: [PATCH 2/2] Print Metal compiler version before running Metal tests This helps pinpoint the issues due to differences in the Metal compiler version and also indicates whether we are actually using the compiler to validate MSL output. --- test_shaders.py | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/test_shaders.py b/test_shaders.py index a3004f41..f3349827 100755 --- a/test_shaders.py +++ b/test_shaders.py @@ -11,6 +11,8 @@ import hashlib import shutil import argparse +METALC = '/Applications/Xcode.app/Contents/Developer/Platforms/iPhoneOS.platform/usr/bin/metal' + def parse_stats(stats): m = re.search('([0-9]+) work registers', stats) registers = int(m.group(1)) if m else 0 @@ -62,9 +64,7 @@ def get_shader_stats(shader): return parse_stats(returned) def validate_shader_msl(shader): - path = "/Applications/Xcode.app/Contents/Developer/Platforms/iPhoneOS.platform/usr/bin/metal" - if os.path.exists(path): - subprocess.check_call([path, '-x', 'metal', '-std=ios-metal1.0', '-Werror', shader]) + subprocess.check_call([METALC, '-x', 'metal', '-std=ios-metal1.0', '-Werror', shader]) def cross_compile_msl(shader): spirv_f, spirv_path = tempfile.mkstemp() @@ -76,7 +76,8 @@ def cross_compile_msl(shader): subprocess.check_call([spirv_cross_path, '--entry', 'main', '--output', msl_path, spirv_path, '--metal']) subprocess.check_call(['spirv-val', spirv_path]) - validate_shader_msl(msl_path) + if os.path.exists(METALC): + validate_shader_msl(msl_path) return (spirv_path, msl_path) @@ -279,6 +280,9 @@ def main(): sys.stderr.write('Need shader folder.\n') sys.exit(1) + if os.path.exists(METALC): + subprocess.check_call([METALC, '--version']) + test_shaders(args.folder, args.update, args.malisc, args.keep, 'metal' if args.metal else 'glsl') if args.malisc: print('Stats in stats.csv!')