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.02k stars 558 forks source link

HLSL->SPIRV->GLSL gives Messy/unoptimized code #1967

Open GregSlazinski opened 2 years ago

GregSlazinski commented 2 years ago

See the original issues: https://github.com/microsoft/DirectXShaderCompiler/issues/2401 https://github.com/microsoft/DirectXShaderCompiler/issues/2402 https://github.com/KhronosGroup/SPIRV-Tools/issues/2816 https://github.com/KhronosGroup/SPIRV-Tools/issues/2817

in short:

the code

VecH4 v;
   v.rgb=Img.Sample(SamplerDefault, tex).rgb*col.rgb;
   v.w=VtxHeightmap;
   return v;

gets unoptimized into:

vec4 _37;

void main()
{
    vec3 _17 = texture(Img, IO1).xyz * IO0.xyz;
    vec4 _20 = vec4(_17.x, _17.y, _17.z, _37.w);
    _20.w = VtxHeightmap;
    RT0 = _20;
}

it reads an invalid value _37.w which could be optimized into: vec4 _20 = vec4(_17.x, _17.y, _17.z, VtxHeightmap); or:

{
    vec4 _20 = vec4(texture(Img, IO1).xyz * IO0.xyz, VtxHeightmap);
    RT0 = _20;
}

or RT0 = vec4(texture(Img, IO1).xyz * IO0.xyz, VtxHeightmap);


AND SIMILAR PROBLEM

Vec4 Test_VS(VtxInput vtx, out MatrixH3 mtrx:MTRX):SV_Position
{
   mtrx[0]=vtx.nrm();
   mtrx[1]=vtx.tan();
   mtrx[2]=vtx.bin(0, 0);
   return 0;
}

-> SPIRV -> GLSL

mat3 _43;

void main()
{
    mat3 _45 = _43;
    _45[0] = ATTR2;
    mat3 _46 = _45;
    _46[1] = ATTR3.xyz;
    mediump vec3 _23 = cross(vec3(0.0), vec3(0.0)) * ATTR3.w;
    mat3 _47 = _46;
    _47[2] = vec3(_23.x, _23.y, _23.z);
    gl_Position = vec4(0.0);
    IO0 = _47;
}

Problem: in HLSL I setup only 1 matrix, on GLSL modifying it results in a new matrix object being created every time. Also it's initialized at the start from an invalid value mat3 _45 = _43;

HansKristian-Work commented 2 years ago

Did you try latest SPIRV-Cross master? This should have been fixed already. If that still doesn't do it, please attach SPIR-V so I can reproduce.

GregSlazinski commented 2 years ago

Updated to latest SPIRV-Cross source from GitHub - Unfortunately it's still broken. There's only 1 improvement, however there's lots of other problems still.

Compiling HLSL:

#define Bool     bool
#define Int      int
#define UInt     uint
#define Flt      float
#define VecI2    int2
#define VecI     int3
#define VecI4    int4
#define VecU2    uint2
#define VecU     uint3
#define VecU4    uint4
#define Vec2     float2
#define Vec      float3
#define Vec4     float4
#define Matrix3  float3x3
#define Matrix   float4x3
#define Matrix4  float4x4

#define Half     min16float
#define VecH2    min16float2
#define VecH     min16float3
#define VecH4    min16float4
#define MatrixH3 min16float3x3
#define MatrixH  min16float4x3
#define MatrixH4 min16float4x4

cbuffer cbuf
{
   bool VtxSkinning;
   Half VtxHeightmap;
};
struct VtxInput // Vertex Input, use this class to access vertex data in vertex shaders
{
   // !! LOC, ATTR numbers AND list order, must be in sync with GL_VTX_SEMANTIC !!
   Vec4  _pos     :ATTR0 ;
   VecH  _hlp     :ATTR1 ;
   VecH  _nrm     :ATTR2 ;
   VecH4 _tan     :ATTR3 ;
   Vec2  _tex     :ATTR4 ;
   Vec2  _tex1    :ATTR5 ;
   Vec2  _tex2    :ATTR6 ;
   Half  _size    :ATTR7 ;
   Vec4  _bone    :ATTR8 ; // this has to be Vec4 because VecI4 and VecU4 don't work for some reason
   Vec4  _weight  :ATTR9 ; // this has to be Vec4 instead of VecH4 because of 2 reasons, we need sum of weights to be equal to 1.0 (half's can't do that), also when converting to GLSL the explicit casts to "Vec weight" precision are optimized away and perhaps some GLSL compilers may want to perform optimizations where Half*Vec is converted to VecH which would destroy precision for skinned characters
   VecH4 _material:ATTR10;
   VecH4 _color   :ATTR11;
   UInt  _instance:SV_InstanceID;

   VecH  nrm      (                                        ) {return _nrm                                                                  ;} // vertex normal
   VecH  tan      (VecH nrm          , Bool heightmap=false) {return heightmap ? VecH(1-nrm.x*nrm.x, -nrm.y*nrm.x, -nrm.z*nrm.x) : _tan.xyz;} // vertex tangent, for heightmap: PointOnPlane(Vec(1,0,0), nrm()), Vec(1,0,0)-nrm*nrm.x, which gives a perpendicular however not Normalized !!
   VecH  bin      (VecH nrm, VecH tan, Bool heightmap=false) {return heightmap ? cross(nrm, tan) : cross(nrm, tan)*_tan.w                  ;} // binormal from transformed normal and tangent
   Vec2  pos2     (                                        ) {return _pos.xy                                                               ;} // vertex position
   Vec   pos      (                                        ) {return _pos.xyz                                                              ;} // vertex position
   Vec4  pos4     (                                        ) {return _pos                                                                  ;} // vertex position in Vec4(pos.xyz, 1) format
   Flt   posZ     (                                        ) {return _pos.z                                                                ;} // vertex position Z
   VecH  hlp      (                                        ) {return _hlp                                                                  ;} // helper position
   VecH  tan      (                                        ) {return _tan.xyz                                                              ;} // helper position
   Vec2  tex      (                    Bool heightmap=false) {return heightmap ? _pos.xz*Vec2(VtxHeightmap, -VtxHeightmap) : _tex          ;} // tex coords 0
   Vec2  tex1     (                                        ) {return                                                         _tex1         ;} // tex coords 1
   Vec2  tex2     (                                        ) {return                                                         _tex2         ;} // tex coords 2
   VecU  bone     (                                        ) {return VtxSkinning ? VecU(_bone.xyz) : VecU(0, 0, 0)                         ;} // bone matrix indexes
};
Texture2D<VecH4> Img;
sampler SamplerLinearClamp;

void DrawUV_VS(VtxInput vtx,
   out VecH4 col:COLOR,
   out Vec2 uv   :UV,
   out float3x3 mtrx:MTRX,
   Vec4 pixel:POSITION)
{
   col=0;
   uv   =vtx.tex();
   mtrx[0] = vtx._nrm;
   mtrx[1] = vtx._tan.xyz;
   mtrx[2] = 0;
   pixel=Vec4(vtx.pos2(), 0, 1); // set Z to be at the end of the viewport, this enables optimizations by processing only foreground pixels (no sky/background)
}

VecH4 Test_PS(VecH4 col:COLOR, Vec2 tex:UV, float3x3 mtrx:MTRX):SV_Target0
{
   VecH4 v;
   v.rgb=Img.Sample(SamplerLinearClamp, tex).rgb*col.rgb;
   v.w=VtxHeightmap;
   return v;
}

DrawUV_VS - Vertex Shader Test_PS - Pixel Shader

gives GLSL

Vertex Shader:

layout(location=0)in vec4 ATTR0;
layout(location=1)in mediump vec3 ATTR1;
layout(location=2)in mediump vec3 ATTR2;
layout(location=3)in mediump vec4 ATTR3;
layout(location=4)in vec2 ATTR4;
layout(location=5)in vec2 ATTR5;
layout(location=6)in vec2 ATTR6;
layout(location=7)in mediump float ATTR7;
layout(location=8)in vec4 ATTR8;
layout(location=9)in vec4 ATTR9;
layout(location=10)in mediump vec4 ATTR10;
layout(location=11)in mediump vec4 ATTR11;
layout(location=12)in vec4 ATTR12;
out vec4 IO0;
out vec2 IO1;
out mat3 IO2;
mat3 _41;
vec2 _42;
void main()
{
mat3 _46;
_46[0]=ATTR2;
_46[1]=ATTR3.xyz;
_46[2]=vec3(0.0);
IO0=vec4(0.0);
IO1=vec2(ATTR4.x,ATTR4.y);
IO2=_46;
}

this is OK mtrx[0] = vtx._nrm; mtrx[1] = vtx._tan.xyz; mtrx[2] = 0; as mat3 _46 is good, but there are useless/unused temporaries generated:

mat3 _41;
vec2 _42;

Pixel Shader:

precision mediump float;
precision highp int;
layout(row_major,std140)uniform _cbuf
{
uint VtxSkinning;
float VtxHeightmap;
};
uniform mediump sampler2D S2_Img;
in vec4 IO0;
in highp vec2 IO1;
in highp mat3 IO2;
layout(location=0)out vec4 RT0;
vec4 _40;
void main()
{
vec3 _18=texture(S2_Img,IO1).xyz*IO0.xyz;
vec4 _19=vec4(_18.x,_18.y,_18.z,_40.w);
_19.w=VtxHeightmap;
RT0=_19;
}

This

   VecH4 v;
   v.rgb=Img.Sample(SamplerLinearClamp, tex).rgb*col.rgb;
   v.w=VtxHeightmap;
   return v;

is still broken as before

vec3 _18=texture(S2_Img,IO1).xyz*IO0.xyz;
vec4 _19=vec4(_18.x,_18.y,_18.z,_40.w);
_19.w=VtxHeightmap;
RT0=_19;
HansKristian-Work commented 2 years ago

If this is using OpCompositeInsert, this is expected and should be optimized at SPIR-V level. SPIRV-Cross is not an optimizer.