OpAtomicSMax behaves as unsigned

[giomasce]

This was initially reported as https://github.com/KhronosGroup/MoltenVK/issues/2194, where it was suggested to file it here.

It seems that the MSL 3.1 code generated for OpAtomicSMax treats the operation as unsigned rather than signed. I'm testing with the SPIR-V code (compiled for Vulkan 1.0):

; SPIR-V
; Version: 1.0
; Generator: Khronos SPIR-V Tools Assembler; 0
; Bound: 18
; Schema: 0
               OpCapability Shader
               OpCapability ImageBuffer
          %1 = OpExtInstImport "GLSL.std.450"
               OpMemoryModel Logical GLSL450
               OpEntryPoint GLCompute %main "main"
               OpExecutionMode %main LocalSize 1 1 1
               OpSource GLSL 450
               OpName %main "main"
               OpName %u0 "u0"
               OpDecorate %u0 DescriptorSet 0
               OpDecorate %u0 Binding 0
       %void = OpTypeVoid
          %5 = OpTypeFunction %void
       %uint = OpTypeInt 32 0
          %7 = OpTypeImage %uint Buffer 0 0 0 2 R32ui
%_ptr_UniformConstant_7 = OpTypePointer UniformConstant %7
         %u0 = OpVariable %_ptr_UniformConstant_7 UniformConstant
        %int = OpTypeInt 32 1
      %int_0 = OpConstant %int 0
%uint_4294967295 = OpConstant %uint 4294967295
     %uint_0 = OpConstant %uint 0
%_ptr_Image_uint = OpTypePointer Image %uint
     %uint_1 = OpConstant %uint 1
       %main = OpFunction %void None %5
         %15 = OpLabel
         %16 = OpImageTexelPointer %_ptr_Image_uint %u0 %int_0 %uint_0
         %17 = OpAtomicSMax %uint %16 %uint_1 %uint_0 %uint_4294967295
               OpReturn
               OpFunctionEnd

With MSL 3.1 this becomes:

% ./spirv-cross --msl shader.spv --msl-version 30100
#pragma clang diagnostic ignored "-Wunused-variable"

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

using namespace metal;

kernel void main0(texture2d<uint, access::read_write> u0 [[texture(0)]])
{
    uint _17 = uint(u0.atomic_fetch_max(uint(0), int(4294967295u)).x);
}

I guess that here the implicit type for atomic_fetch_max is taken from the resource, not from the argument, as it would happen with usual C++ atomics.

For MSL 3.0 this is generated instead:

% ./spirv-cross --msl shader.spv --msl-version 30000
#pragma clang diagnostic ignored "-Wunused-variable"

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

using namespace metal;

kernel void main0(texture2d<uint> u0 [[texture(0)]], device atomic_uint* u0_atomic [[buffer(0)]])
{
    uint _17 = uint(atomic_fetch_max_explicit((device atomic_int*)&u0_atomic[0], int(4294967295u), memory_order_relaxed));
}

This looks correct AFAICT, and works properly on MoltenVK.

[HansKristian-Work]

Is this fixable? Does it work if you reinterpret_cast texture2d<uint, access::read_write> to texture2d<int, access::read_write>?

[HansKristian-Work]

Seems like this works ...

#pragma clang diagnostic ignored "-Wunused-variable"

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

using namespace metal;

template <typename T, typename U>
static inline T texture_cast(U img)
{
    return reinterpret_cast<thread const T &>(img);
}

kernel void main0(texture2d<uint, access::read_write> u0 [[texture(0)]])
{
    uint _17 = uint(texture_cast<texture2d<int, access::read_write>>(u0).atomic_fetch_max(uint2(0, 1), int(4294967295u)).x);
}

; Function Attrs: mustprogress nounwind willreturn
define void @main0(%struct._texture_2d_t addrspace(1)* nocapture %0) local_unnamed_addr #0 {
  %2 = tail call <4 x i32> @air.atomic_fetch_max_explicit_texture_2d.s.v4i32(%struct._texture_2d_t addrspace(1)* nocapture %0, <2 x i32> <i32 0, i32 1>, <4 x i32> <i32 -1, i32 -1, i32 -1, i32 -1>, i32 0, i32 3) #2
  ret void
}