Skip to content
Snippets Groups Projects

vkd3d-shader/dxil: Emit the shader instructions.

Merged Conor McCarthy requested to merge cmccarthy/vkd3d:dxil_instructions into master
2 unresolved threads

Sufficient for compiling a no-op pixel shader.

This should probably be rebased on top of !263 (closed) because it introduces vkd3d_spirv_get_type_id_for_data_type(), which 263 renders unnecessary.

!263 (closed) is not essential, but I think using two different type systems in the backend is not ideal.

Merge request reports

Merge request pipeline #12789 skipped

Merge request pipeline skipped for c0a2bb10

Approval is optional

Merged by Alexandre JulliardAlexandre Julliard 1 year ago (Jul 20, 2023 8:57pm UTC)

Merge details

  • Changes merged into with c0a2bb10.
  • Deleted the source branch.

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
  • Conor McCarthy added 25 commits

    added 25 commits

    • 5ac568bf...e0e261ea - 19 commits from branch wine:master
    • 7d3f352a - vkd3d-shader/dxil: Return an error from sm6_parser_globals_init() on invalid operand count.
    • aa793570 - vkd3d-shader/spirv: Do not normalise Shader Model 6 shaders.
    • 937efda6 - vkd3d-shader/dxil: Emit the shader instructions.
    • 239e5192 - vkd3d-shader/dxbc: Allow DXBC containers to have a zero checksum.
    • 9e926527 - vkd3d-shader/spirv: Introduce an undefined register type.
    • f33e8354 - vkd3d-shader/dxil: Emit undefined constants.

    Compare with previous version

    • @@ -2428,6 +2429,7 @@ static struct spirv_compiler *spirv_compiler_create(const struct vkd3d_shader_ve
           rb_init(&compiler->symbol_table, vkd3d_symbol_compare);
       
           compiler->shader_type = shader_version->type;
      +    compiler->is_dxil = shader_version->major >= 6;
       
           if ((shader_interface = vkd3d_find_struct(compile_info->next, INTERFACE_INFO)))
           {
      @@ -9480,7 +9482,7 @@ static int spirv_compiler_generate_spirv(struct spirv_compiler *compiler,
           compiler->location.column = 0;
           compiler->location.line = 1;
       
      -    if ((result = vkd3d_shader_normalise(parser)) < 0)
      +    if (!compiler->is_dxil && (result = vkd3d_shader_normalise(parser)) < 0)
               return result;
       
           instructions = parser->instructions;

      I think we can just do that check inside vkd3d_shader_normalise(), and that seems preferable.

      +static bool sm6_block_emit_instructions(struct sm6_block *block, struct sm6_parser *sm6)
      +{
      +    /* Reserve an extra instruction for the block terminator. */
      +    struct vkd3d_shader_instruction *ins = sm6_parser_require_space(sm6, block->instruction_count + 1);
      +
      +    if (!ins)
      +        return false;
      +
      +    memcpy(ins, block->instructions, block->instruction_count * sizeof(*block->instructions));
      +    sm6->p.instructions.count += block->instruction_count;
      +    return true;
      +}

      Could we append that instruction here, instead of in the caller? Or as part of parsing the block, even?

      From 239e51923cf60b33d4dddada3fadaaf7b769ed94 Mon Sep 17 00:00:00 2001
      From: Conor McCarthy <cmccarthy@codeweavers.com>
      Date: Wed, 17 May 2023 16:43:26 +1000
      Subject: [PATCH 4/6] vkd3d-shader/dxbc: Allow DXBC containers to have a zero
       checksum.
      
      The linux build of DXC writes a zero checksum.

      But as far as I'm aware, such shaders are rejected by native d3d12. What are we trying to do here, exactly? If it's purely about local development testing with the Linux build of DXC, I have a tool for inspecting and modifying DXBC blobs that I've been meaning to upstream.

    • Author Developer

      What are we trying to do here, exactly?

      Yes it is for tests added later. I've deleted it from this series. We will need a means to add the signature for the tests.

      For the d3d12 and shader runner tests I've been using an environment variable to specify the path to DXC, HLSL source declarations in d3d12.c, and an [enable] directive in shader runner. Are those acceptable for upstream?

      Edited by Conor McCarthy
    • Please register or sign in to reply
  • Conor McCarthy added 4 commits

    added 4 commits

    • 9f6ce8d3 - vkd3d-shader/spirv: Do not normalise Shader Model 6 shaders.
    • f0a08778 - vkd3d-shader/dxil: Emit the shader instructions.
    • ed6bd043 - vkd3d-shader/spirv: Introduce an undefined register type.
    • 1bdcee0e - vkd3d-shader/dxil: Emit undefined constants.

    Compare with previous version

    • For the d3d12 and shader runner tests I've been using an environment variable to specify the path to DXC, HLSL source declarations in d3d12.c, and an [enable] directive in shader runner. Are those acceptable for upstream?

      It's of course not ideal to depend on DXC/dxcompiler for the DXIL shader runner tests like that, but I think we can work with that, yes. There's some precedent as well, because we can also use d3dcompiler for running shader runner tests.

      In terms of the checksum, the most practical way to make that work in the shader runner is probably to introduce a flag like "VKD3D_SHADER_PARSE_DXBC_IGNORE_CHECKSUM" for vkd3d_shader_parse_dxbc(), then use that to parse the DXBCs produced by Linux DXC, and use vkd3d_shader_serialize_dxbc() to create a DXBC blob to pass to d3d12/vkd3d. Also, would we need to use DXC, or could we use the dxcompiler library?

    • Author Developer

      Using dxcompiler would be preferable, but instead of COM interfaces it uses C++ linking, and when I tried using it a while ago I couldn't get the calling conventions to match up, even if the C declaration was explicitly declared the same convention as the dxcompiler function. It worked if I added a wrapper in a C++ source file, but we probably don't want that. I'll take another look and see if they can be made to match up.

    • Please register or sign in to reply
  • Henri Verbeet approved this merge request

    approved this merge request

  • Giovanni Mascellani approved this merge request

    approved this merge request

  • added 5 commits

    • 8fe6d2ee - vkd3d-shader/dxil: Return an error from sm6_parser_globals_init() on invalid operand count.
    • f26d4758 - vkd3d-shader/spirv: Do not normalise Shader Model 6 shaders.
    • 3e553aaa - vkd3d-shader/dxil: Emit the shader instructions.
    • 402c93fa - vkd3d-shader/spirv: Introduce an undefined register type.
    • c0a2bb10 - vkd3d-shader/dxil: Emit undefined constants.

    Compare with previous version

  • Alexandre Julliard approved this merge request

    approved this merge request

Please register or sign in to reply
Loading