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

Reflection gets confused by DXC's debug outputs #38

Closed
farnoy opened this issue Sep 7, 2020 · 2 comments
Closed

Reflection gets confused by DXC's debug outputs #38

farnoy opened this issue Sep 7, 2020 · 2 comments

Comments

@farnoy
Copy link

farnoy commented Sep 7, 2020

Hi, I saw your announcement on /r/rust_gamedev and tried out your library to replace spirv_reflect. I love the API and Debug instances, the types in spirq::ty are also idiomatic to work with!

I have a problem where enabling -Zi and/or -fspv-reflect affects the reflection (negatively in both cases). I don't need these for now so I'm good, but it would be a good idea to look into why these annotations break things. From what I could gather, -fspv-reflect only added a couple OpDecorateString entries which caused spirq to not see any inputs for that entrypoint.

I'm attaching my shader, dxc compile options and Debug output from spirq:

struct PushConstants {
  float2 scale;
  float2 translate;
};

[[vk::push_constant]] PushConstants pushConstants;

float4 main(in float2 pos : POSITION,
            in float2 uv : TEXCOORD,
            in float4 col : COLOR,
            out float4 outColor : COLOR,
            out float2 outUv : TEXCOORD): SV_POSITION {
  outColor = col;
  outUv = uv;
  float4 outPos = float4(pos * pushConstants.scale + pushConstants.translate, 0, 1);
  outPos.y *= -1.0;
  return outPos;
}
$ dxc -spirv -T vs_6_0 -E main -fspv-target-env=vulkan1.1 -Fo whatever.spv

Debug output:

    main {
        push_const: Some(
            { scale: vec2<f32>, translate: vec2<f32> },
        ),
        inputs: {
            (loc=0, comp=0): vec2<f32>,
            (loc=1, comp=0): vec2<f32>,
            (loc=2, comp=0): vec4<f32>,
        },
        outputs: {
            (loc=0, comp=0): vec4<f32>,
            (loc=1, comp=0): vec2<f32>,
        },
        descriptors: {},
        spec_consts: {},
    },

Perfect!

Now adding -fspv-reflect as an option to dxc, SPIR-V diff is now:

diff --git a/a.spv b/b.spv
index a3e8063..f22924c 100644
--- a/bare.spv
+++ b/test.spv
@@ -4,6 +4,7 @@
 ; Bound: 42
 ; Schema: 0
                OpCapability Shader
+               OpExtension "SPV_GOOGLE_hlsl_functionality1"
                OpMemoryModel Logical GLSL450
                OpEntryPoint Vertex %main "main" %in_var_POSITION %in_var_TEXCOORD %in_var_COLOR %gl_Position %out_var_COLOR %out_var_TEXCOORD
                OpSource HLSL 600
@@ -17,7 +18,13 @@
                OpName %out_var_COLOR "out.var.COLOR"
                OpName %out_var_TEXCOORD "out.var.TEXCOORD"
                OpName %main "main"
+               OpDecorateString %in_var_POSITION UserSemantic "POSITION"
+               OpDecorateString %in_var_TEXCOORD UserSemantic "TEXCOORD"
+               OpDecorateString %in_var_COLOR UserSemantic "COLOR"
                OpDecorate %gl_Position BuiltIn Position
+               OpDecorateString %gl_Position UserSemantic "SV_POSITION"
+               OpDecorateString %out_var_COLOR UserSemantic "COLOR"
+               OpDecorateString %out_var_TEXCOORD UserSemantic "TEXCOORD"
                OpDecorate %in_var_POSITION Location 0
                OpDecorate %in_var_TEXCOORD Location 1
                OpDecorate %in_var_COLOR Location 2

But the output is:

    main {
        push_const: None,
        inputs: {},
        outputs: {},
        descriptors: {},
        spec_consts: {},
    },

It gets similarly confused by the -Zi flag, but not to such a great extent. It would be nice to support these flags as they're seemingly becoming standard for HLSL on Vulkan debugging and other tools.

@PENGUINLIONG
Copy link
Owner

Glad you like it. :)
It seems spirq is currently not recognizing
OpDecorateStrings. Will fix soon.

@PENGUINLIONG
Copy link
Owner

Thanks for the reproduction details! They made things easy.

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

2 participants