Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

spirv-reflect dumping wrong info for $Global block #48

Closed
nsubtil opened this issue Apr 28, 2018 · 5 comments
Closed

spirv-reflect dumping wrong info for $Global block #48

nsubtil opened this issue Apr 28, 2018 · 5 comments

Comments

@nsubtil
Copy link
Member

nsubtil commented Apr 28, 2018

Input HLSL:

struct VSInput
{
    float2 pos : POSITION;
    float4 color : COLOR0;
    float2 uv : TEXCOORD0;
};

struct VSOutput
{
    float4 pos : SV_POSITION;
    float4 color : COLOR0;
    float2 uv : TEXCOORD0;
};

cbuffer myconstants : register(b0)
{
    float delta;
    float delta2;
};

float himom : register(b1);
float byemom : register(b2);

VSOutput main(VSInput input)
{
    VSOutput o;
    o.pos = float4(input.pos, 0.f, 1.f);
    o.color = input.color + float4(delta, delta2, delta + byemom, delta + himom);
    o.uv = input.uv;

    return o;
}

Compiling with dxc -spirv -fspv-reflect -fspv-target-env=vulkan1.1 and running spirv-reflect on the output, I see the following bit:

  Descriptor bindings: 2
    0:
      binding : 0
      set     : 0
      type    : VK_DESCRIPTOR_TYPE_UNIFORM_BUFFER (CBV)
      name    : myconstants (type.myconstants)
          // offset = 0, abs offset = 0, size = 16, padded size = 16
          struct type.myconstants {
              float delta;          // offset = 0, abs offset = 0, size =  4, padded size =  4
              float delta2;         // offset = 4, abs offset = 4, size =  4, padded size = 12
          } type.myconstants;

    1:
      binding : 1
      set     : 0
      type    : VK_DESCRIPTOR_TYPE_UNIFORM_BUFFER (CBV)
      name    : $Globals (type.myconstants)
          // offset = 0, abs offset = 0, size = 16, padded size = 16
          struct type.myconstants {
              float delta;          // offset = 0, abs offset = 0, size =  4, padded size =  4
              float delta2;         // offset = 4, abs offset = 4, size =  4, padded size = 12
          } type.myconstants;

It looks like it's dumping the myconstants block twice, instead of dumping $Globals the second time around.

@antiagainst
Copy link
Contributor

Hmm, interesting. In DXC, we are trying to reuse types if possible in order to generate smaller SPIR-V. The above is such a case. Note that $Globals is a cbuffer containing two floats, the same as myconstants. So these two cbuffers have exactly the same type, including members, layout, decoration, etc. So we are reusing the struct type for them. That's why we are seeing two variables, one named as myconstant and the other named as $Globals, but they are having the same type type.myconstants. And here is the output from DXC, which will make it clear:

               OpDecorate %myconstants DescriptorSet 0
               OpDecorate %myconstants Binding 0
               OpDecorate %_Globals DescriptorSet 0
               OpDecorate %_Globals Binding 1

%myconstants = OpVariable %_ptr_Uniform_type_myconstants Uniform
   %_Globals = OpVariable %_ptr_Uniform_type_myconstants Uniform

@chaoticbob
Copy link
Contributor

Closing this since it's not a spirv-reflect bug. Will sort it out with DXC.

@s-perron
Copy link
Contributor

@nsubtil Can I know what the actual problem is? We have a similar issue discussed in spir-v tools (KhronosGroup/SPIRV-Tools#1372).

Is the problem that the text from spir-v reflect looks wrong? Or is the problem that another tool wants to do something with the name of the types (names of the members?), and that tool is confused by the output?

I just want to be sure I understand the problem before I make too many changes.

@nsubtil
Copy link
Member Author

nsubtil commented Jun 22, 2018

@s-perron the problem is that the output from SPIRV-Reflect looks wrong --- the two standalone variables (himom and byemom) don't show up at all, and the myconstants block shows up twice.

@antiagainst
Copy link
Contributor

Just wanted to give an update:

microsoft/DirectXShaderCompiler#1385 is merged, which incorporates Steven's fix on spirv-opt: KhronosGroup/SPIRV-Tools#1640. This issue should be fixed now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants