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
1.96k stars 549 forks source link

MSL: Casting from float to short/ushort introduces a cast to half in between. #2307

Closed forbiddencactus closed 2 months ago

forbiddencactus commented 2 months ago

The following HLSL code generates the following MSL when compiled to SPIR-V in DXC and run through SPIR-V Cross:

struct VS_OUTPUT
{ 
    float4 Pos      : SV_Position;
};

PS_OUTPUT ps(VS_OUTPUT inputs)
{
uint16_t2 uv = uint16_t2(inputs.Pos.xy);    
}

Generated MSL: short2 _87 = short2(ushort2(half2(gl_FragCoord.xy)));

Casting from float to either uint16 or int16 generates the same cast to half in between, ruining UV sampling precision in our code.

HansKristian-Work commented 2 months ago

Do you have reproducing SPIR-V? Most likely, this is DXC that generated the pattern in SPIR-V. Make sure that the SPIR-V is actually doing OpConvertFToS directly.

HansKristian-Work commented 2 months ago

Yes, this is broken DXC.

struct VS_OUTPUT
{ 
    float4 Pos      : SV_Position;
};

uint16_t2 main(VS_OUTPUT inputs) : SV_Target
{
    return uint16_t2(inputs.Pos.xy);    
}
       %main = OpFunction %void None %11
         %15 = OpLabel
         %16 = OpLoad %v4float %gl_FragCoord
         %17 = OpVectorShuffle %v2float %16 %16 0 1
         %18 = OpFConvert %v2half %17
         %19 = OpConvertFToU %v2ushort %18
               OpStore %out_var_SV_Target %19
               OpReturn
               OpFunctionEnd

DXIL output is correct:

define void @main() {
  %1 = call float @dx.op.loadInput.f32(i32 4, i32 0, i32 0, i8 0, i32 undef)  ; LoadInput(inputSigId,rowIndex,colIndex,gsVertexAxis)
  %2 = call float @dx.op.loadInput.f32(i32 4, i32 0, i32 0, i8 1, i32 undef)  ; LoadInput(inputSigId,rowIndex,colIndex,gsVertexAxis)
  %3 = fptoui float %1 to i16
  %4 = fptoui float %2 to i16
  call void @dx.op.storeOutput.i16(i32 5, i32 0, i32 0, i8 0, i16 %3)  ; StoreOutput(outputSigId,rowIndex,colIndex,value)
  call void @dx.op.storeOutput.i16(i32 5, i32 0, i32 0, i8 1, i16 %4)  ; StoreOutput(outputSigId,rowIndex,colIndex,value)
  ret void
}

Please file a DXC bug.

forbiddencactus commented 2 months ago

Ooops! My bad, should have checked the SPIR-V output first. Sorry!