KhronosGroup / SPIRV-Cross

SPIRV-Cross is a practical tool and library for performing reflection on SPIR-V and disassembling SPIR-V back to high level languages.
Apache License 2.0
2.01k stars 554 forks source link

[MSL] Is it necessary to use `spvUnsafeArray` type for array on the stack? #2010

Open chirsz-ever opened 2 years ago

chirsz-ever commented 2 years ago

For example, use this glsl code to generate SPIR-V for Vulkan with glslangValidator -V test.frag:

#version 310 es
precision highp float;

layout(location=0) in vec3 color;
layout(location=0) out vec4 fragColor;

void main()
{
    vec2 arr[3] = vec2[3](color.xy, color.yz, color.zx);
    fragColor = vec4(arr[0], arr[1]);
}

Then pass it to SPIRV-Cross with spirv-cross --msl frag.spv, this would generate:

#pragma clang diagnostic ignored "-Wmissing-prototypes"
#pragma clang diagnostic ignored "-Wmissing-braces"

#include <metal_stdlib>
#include <simd/simd.h>

using namespace metal;

template<typename T, size_t Num>
struct spvUnsafeArray
{
    T elements[Num ? Num : 1];

    thread T& operator [] (size_t pos) thread
    {
        return elements[pos];
    }
    constexpr const thread T& operator [] (size_t pos) const thread
    {
        return elements[pos];
    }

    device T& operator [] (size_t pos) device
    {
        return elements[pos];
    }
    constexpr const device T& operator [] (size_t pos) const device
    {
        return elements[pos];
    }

    constexpr const constant T& operator [] (size_t pos) const constant
    {
        return elements[pos];
    }

    threadgroup T& operator [] (size_t pos) threadgroup
    {
        return elements[pos];
    }
    constexpr const threadgroup T& operator [] (size_t pos) const threadgroup
    {
        return elements[pos];
    }
};

struct main0_out
{
    float4 fragColor [[color(0)]];
};

struct main0_in
{
    float3 color [[user(locn0)]];
};

fragment main0_out main0(main0_in in [[stage_in]])
{
    main0_out out = {};
    spvUnsafeArray<float2, 3> _22 = spvUnsafeArray<float2, 3>({ in.color.xy, in.color.yz, in.color.zx });
    spvUnsafeArray<float2, 3> arr;
    arr = _22;
    out.fragColor = float4(arr[0], arr[1]);
    return out;
}

The spvUnsafeArray type looks unnecessary, the better code would be like:

#pragma clang diagnostic ignored "-Wmissing-prototypes"
#pragma clang diagnostic ignored "-Wmissing-braces"

#include <metal_stdlib>
#include <simd/simd.h>

using namespace metal;

struct main0_out
{
    float4 fragColor [[color(0)]];
};

struct main0_in
{
    float3 color [[user(locn0)]];
};

fragment main0_out main0(main0_in in [[stage_in]])
{
    main0_out out = {};
    float2 arr[3] = { in.color.xy, in.color.yz, in.color.zx };
    out.fragColor = float4(arr[0], arr[1]);
    return out;
}
HansKristian-Work commented 2 years ago

Arrays in SPIR-V are value types, but MSL inherits pointer-decay ala C. Copying arrays leads to very poor code in MSL, and the template value type wrapper is a workaround that we are forced to keep for performance. There is a force_native_arrays option for MSL, but it has many edge cases w.r.t. correctness. For trivial array use, it should be fine though ...