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.06k stars 564 forks source link

out-of-scope variable in metal source #2068

Closed nedres closed 1 year ago

nedres commented 1 year ago

Using latest release of dxc & spirv-cross, I'm seeing an out-of-scope variable reference in the metal source.

hlsl:

struct Data  { uint4 data[12]; };
struct Output{ uint  data : SV_TARGET1; }; 
struct MyConsts { uint opt; }; 
cbuffer      scene     : register(b3) { MyConsts myConsts; }
SamplerState mySampler : register(s2);
Texture2D texTable[1] ; 

float GetSample(in Data data) {
  Texture2D tex = texTable[(uint(data.data[11].y & 0xFFFFFF))] ;
  [ branch ]
    if (myConsts.opt != 0) {return 1;} 
    else {return tex.SampleLevel(mySampler, float2(0,0), 0 ).x;} 
}

Output ps_main() { 
  const Data data;
  float3 A = float3( -float2(1,1), GetSample(data).x ) ; 
  float3 B = float3( +float2(1,1), GetSample(data).x ) ; 
  Output frag;
  frag.data = cross(A,B).x;
  return frag ; 
} 

Command lines:

echo "hlsl to spirv text"
$DxcPath/dxc.exe -spirv -Zpc -fvk-use-dx-layout -fvk-use-dx-position-w -T ps_6_0 -E ps_main A.hlsl > A.txt

echo "hlsl to spirv binary"
$DxcPath/dxc.exe -spirv -Zpc -fvk-use-dx-layout -fvk-use-dx-position-w -T ps_6_0 -E ps_main A.hlsl -Fo A.spv

echo "spirv to metal"
$SpxPath/spirv-cross.exe  A.spv --output A.metal --msl --msl-ios --msl-version 20300 --msl-texture-buffer-native

echo "metal to air"
$MtlPath/metal.exe -std=ios-metal2.3 -gline-tables-only -MO -c A.metal -o A.air

output of metal.exe:

C:/Users/nresnikoff/A.metal:53:28: error: use of undeclared identifier '_42'; did you mean '_47'?
            _66 = texTable[_42].sample(mySampler, float2(0.0), level(0.0)).x;
                           ^~~
                           _47
C:/Users/nresnikoff/A.metal:26:10: note: '_47' declared here
    bool _47;
         ^
1 error generated.

spirv source: A.txt

generated metal source: A.metal.txt

HansKristian-Work commented 1 year ago

This is pretty hairy, yes ... The OpImage is loaded once inside the first loop construct, and reused later. We have to propagate the implied usage of the _42 index somehow.