KhronosGroup / SPIRV-Tools

Apache License 2.0
1.09k stars 559 forks source link

spirv-cross: float array struct member converted to float4 array but incorrectly accessed #5877

Closed bejado closed 1 week ago

bejado commented 1 week ago

When cross-compiling to MSL via spirv-cross, the float myLod[6] member in the MaterialParameters struct is converted from a float[6] to a float4[6], but accessed without adding a required .x suffix. As a result, the Metal compiler fails due to mismatched types.

shader.metal:19:67: error: no matching conversion for functional-style cast from 'const constant float4' (vector of 4 'float' values) to 'metal::level'
    out.fragColor = mySampler.sample(mySamplerSmplr, float2(0.0), level(materialParams.myLod[0]));

Normally the .x suffix is added, but there appears to be a bug when using it as an argument to textureLod.

See the attached input.frag.txt.

Repro steps:

mv input.frag.txt input.frag

# Compile
glslangValidator -V -o compiled.spv input.frag

# Cross-compile to Metal
spirv-cross --msl compiled.spv > shader.metal

# Cross-compiling to GLSL works correctly
# spirv-cross compiled.spv

# Compiling with Apple's Metal shader compiler fails
xcrun -sdk "iphoneos" metal -c shader.metal -o frag.air -Os -arch air64 -emit-llvm -mios-version-min=15.0

The intermediate SPIR-V looks correct:

%_arr_float_uint_6 = OpTypeArray %float %uint_6
%MaterialParams = OpTypeStruct %_arr_float_uint_6

spirv-cross version:

Git commit: vulkan-sdk-1.3.296.0-32-ga0183472 Timestamp: 2024-11-11T16:57:49
bejado commented 1 week ago

This patch fixes it, but is probably incomplete.

diff --git a/spirv_msl.cpp b/spirv_msl.cpp
index e38959d6..4b1d7fdb 100644
--- a/spirv_msl.cpp
+++ b/spirv_msl.cpp
@@ -11898,7 +11898,7 @@ string CompilerMSL::to_function_args(const TextureFunctionArguments &args, bool
                }
                else
                {
-                       farg_str += ", level(" + to_expression(lod) + ")";
+                       farg_str += ", level(" + to_unpacked_expression(lod) + ")";
                }
        }
        else if (args.base.is_fetch && !lod && (imgtype.image.dim != Dim1D || msl_options.texture_1D_as_2D) &&
s-perron commented 1 week ago

You will need to open this in https://github.com/KhronosGroup/SPIRV-Cross. Even though it is a tool that works with SPIR-V, spirv-cross is not part of spirv-tools.

bejado commented 1 week ago

Whoops, my mistake thank you!

For reference: https://github.com/KhronosGroup/SPIRV-Cross/issues/2416