[Bug 49186] New: DXBC compiler generates incorrect spir-v code

[WineHQ Bugzilla]

https://bugs.winehq.org/show_bug.cgi?id=49186

Bug ID: 49186
           Summary: DXBC compiler generates incorrect spir-v code
           Product: vkd3d
           Version: 1.1
          Hardware: x86
                OS: Linux
            Status: UNCONFIRMED
          Severity: normal
          Priority: P2
         Component: vkd3d
          Assignee: [e-mail]
          Reporter: [e-mail]
      Distribution: ---

Created attachment 67204
  --> https://bugs.winehq.org/attachment.cgi?id=67204
dxbc tessellaton control shader

I'm having issues with tessellation shaders.
Seems that the generated spir-v code is incorrect. After converting the spir-v
to GLSL (spirv-dis) and validating it (glslangValidator) there are errors in
the the following section:

#version 450
layout(vertices = 5) out;
out float gl_ClipDistance[5];
layout(location = 0) out vec2 o0[5];
layout(location = 1) out vec4 o1[5];
vec4 opc[4];
vec4 v1[5];
void control0()
{
    v1[1u] = vec4(gl_ClipDistance[0u]);
    v1[2u] = vec4(gl_ClipDistance[1u]);
    v1[3u] = vec4(gl_ClipDistance[2u]);
    v1[4u] = vec4(gl_ClipDistance[3u]);
    v1[5u] = vec4(gl_ClipDistance[4u]);
    vec4 r0;
    r0.x = intBitsToFloat(gl_InvocationID);
    o0[gl_InvocationID] = gl_in[floatBitsToInt(r0.x)].gl_Position.xy;
    o1[gl_InvocationID] = v1[floatBitsToInt(r0.x)];
}
...

glslangValidator outputs the following error:
ERROR: 0:4: 'gl_ClipDistance' : identifiers starting with "gl_" are reserved
ERROR: 1 compilation errors.  No code generated.

Sample DXBC binary is attached

[Bug 49186] DXBC compiler generates incorrect spir-v code

[WineHQ Bugzilla]

https://bugs.winehq.org/show_bug.cgi?id=49186

--- Comment #1 from Henri Verbeet  ---
Created attachment 67672
  --> https://bugs.winehq.org/attachment.cgi?id=67672
patch

It took me a while to get to this, but does the attached patch help? It may not
be fully correct; needs tests.

Also, how was hull0.fxc created? It has an invalid DXBC checksum (all zeroes).
Currently vkd3d-shader does not validate DXBC checksums, but it will in the
future.

[Bug 49186] DXBC compiler generates incorrect spir-v code

[WineHQ Bugzilla]

https://bugs.winehq.org/show_bug.cgi?id=49186

--- Comment #2 from Alex  ---
The patch doesn't help, although I didn't look at the produced shader binary.
Perhaps those error messages are relevant:

fixme:vkd3d_dxbc_compiler_check_index_range: Unhandled index range write mask
0x1 (0xf).
fixme:vkd3d_dxbc_compiler_emit_dcl_index_range: Ignoring dcl_index_range 0x6 2.

[Bug 49186] DXBC compiler generates incorrect spir-v code

[WineHQ Bugzilla]

https://bugs.winehq.org/show_bug.cgi?id=49186

--- Comment #3 from Henri Verbeet  ---
(In reply to Alex from comment #2)
> The patch doesn't help, although I didn't look at the produced shader binary.

I'm afraid I may need some more context on what you're trying to do in order to
reproduce the issue then. For what it's worth:

The issue the patch addresses is the following (from spirv-val):

error: line 130: OpAccessChain result type (OpTypeVector) does not match
the type that results from indexing into the base <id> (OpTypeFloat).
      %52 = OpAccessChain %_ptr_Input_v4float %v1 %51

That happens because of the following bits in the .fxc:

// Input signature:
    [...]
    // SV_CLIPDISTANCE          0   xyzw        1  CLIPDST   float
    [...]
    dcl_input v[5][1].xyzw
    [...]
    mov o1.xyzw, v[r0.x + 0][1].xyzw

v1 is a vec4 bound to SV_CLIPDISTANCE in the .fxc, but SpvBuiltInClipDistance
is a scalar. The patch handles that difference by filling the additional
components (.yzw) with zeroes; it may need to replicate the .x component
instead, that's the part that needs tests.

It's not entirely clear to me how you produced the quoted GLSL. You mention
spirv-dis, but at least the version I have here doesn't appear to have that
feature. (But, it's also more than two weeks old...) I know spirv-cross can
turn SPIR-V into GLSL, is that perhaps what you used?

In any case, the following line in the quoted GLSL appears to have been
incorrectly translated from the SPIR-V:

out float gl_ClipDistance[5];

Specifically, the "out" there. From the SPIR-V:

OpDecorate %v1 BuiltIn ClipDistance
    [...]
    %v1 = OpVariable %_ptr_Input__arr_float_uint_5 Input

ClipDistance is used as an input, not an output.
> Perhaps those error messages are relevant:
>
> fixme:vkd3d_dxbc_compiler_check_index_range: Unhandled index range write
> mask 0x1 (0xf).
> fixme:vkd3d_dxbc_compiler_emit_dcl_index_range: Ignoring dcl_index_range 0x6
> 2.

That's from this line in the .fxc:

dcl_indexrange o0.x 2

In context:

hs_fork_phase 
    dcl_hs_fork_phase_instance_count 2
    dcl_input vForkInstanceID
    dcl_output_siv o0.x, finalTriUeq0EdgeTessFactor
    dcl_output_siv o1.x, finalTriVeq0EdgeTessFactor
    dcl_temps 1
    dcl_indexrange o0.x 2
    mov r0.x, vForkInstanceID.x
    mov o[r0.x + 0].x, l(1.000000)
    ret

We should of course fix that, but it seems unlikely to be consequential for the
purposes of this bug report.

[Bug 49186] DXBC compiler generates incorrect spir-v code

[WineHQ Bugzilla]

https://bugs.winehq.org/show_bug.cgi?id=49186

--- Comment #4 from Alex  ---
I must admit that I'm totally out of my depth here. I don't have any knowledge
(just yet) of neither spir-v nor GLSL and those are only small bits and pieces
of my project. I'm working on a virtual GPU device capable of running Windows
VMs with DirectX 11/12 support on top of Linux. Basically it involves
translating DirectX APIs and shaders coming from Windows GPU driver into Vulkan
commands. This is where vkd3d comes in very handy.
Now, regarding the shaders: unfortunately Windows doesn't provide the driver
with a complete shader dxbc binary as vkd3d compiler expects, instead I receive
a binary code section and input/output signatures. From those I construct a
complete dxbc shader that goes into vkd3d compiler and then into the vulkan
pipeline. While Vertex and Pixel shaders work more or less, tessellation ones
do not, causing instead a crash somewhere inside intel vulkan drivers. This is
where I started my investigation, I collected dxbc shaders and compiled it with
vkd3d, then disassembled and converted to GLSL for analysis with spirv-tools,
hence my bug report.
I'm very grateful for you detailed explanation of what's going on. Currently I 
lack the knowledge to process it properly but I'm going to dive in into this
stuff in a few weeks time after dealing with more urgent stuff.
Again, thank you for your time and efforts. Please don't close this issue just
yet, I'll be in touch once I have more information.

[Bug 49186] DXBC compiler generates incorrect spir-v code

[WineHQ Bugzilla]

https://bugs.winehq.org/show_bug.cgi?id=49186

--- Comment #5 from Henri Verbeet  ---
Tessellation shader shader support is still a little less solid than the other
shader types, so in that regard it's certainly plausible that there could be
bugs in that area.

In case it's useful to you, note that vkd3d_compute_dxbc_checksum() can be used
to compute DXBC checksums. That's currently an internal function, but we will
likely expose DXBC manipulation interfaces in a future release.

In any case, thank you for the bug report.

[Bug 49186] DXBC compiler generates incorrect spir-v code

[WineHQ Bugzilla]

https://bugs.winehq.org/show_bug.cgi?id=49186

François Gouget  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |patch
                 CC|                            |[e-mail]