[PATCH 0/2] MR44: vkd3d-shader/dxil: Read and validate DXIL bitcode blocks.

[Conor McCarthy (＠cmccarthy)]

Block records are not processed; only the bitcode is validated.

-- 
https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/44

[PATCH 1/2] vkd3d-shader/dxil: Read and validate DXIL bitcode blocks.

[Conor McCarthy]

From: Conor McCarthy <cmccarthy(a)codeweavers.com>

Block records are not processed; only the bitcode is validated.
---
 Makefile.am                              |   2 +
 include/vkd3d_shader.h                   |   5 +
 libs/vkd3d-shader/dxbc.c                 |  14 +-
 libs/vkd3d-shader/dxil.c                 | 813 +++++++++++++++++++++++
 libs/vkd3d-shader/sm6.h                  |  64 ++
 libs/vkd3d-shader/trace.c                |   6 +
 libs/vkd3d-shader/vkd3d_shader_main.c    |  51 +-
 libs/vkd3d-shader/vkd3d_shader_private.h |   9 +
 8 files changed, 958 insertions(+), 6 deletions(-)
 create mode 100644 libs/vkd3d-shader/dxil.c
 create mode 100644 libs/vkd3d-shader/sm6.h

diff --git a/Makefile.am b/Makefile.am
index 1340be10..843fb4f8 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -234,6 +234,7 @@ libvkd3d_shader_la_SOURCES = \
 	libs/vkd3d-shader/checksum.c \
 	libs/vkd3d-shader/d3dbc.c \
 	libs/vkd3d-shader/dxbc.c \
+	libs/vkd3d-shader/dxil.c \
 	libs/vkd3d-shader/glsl.c \
 	libs/vkd3d-shader/hlsl.c \
 	libs/vkd3d-shader/hlsl.h \
@@ -243,6 +244,7 @@ libvkd3d_shader_la_SOURCES = \
 	libs/vkd3d-shader/hlsl_sm4.c \
 	libs/vkd3d-shader/preproc.h \
 	libs/vkd3d-shader/sm4.h \
+	libs/vkd3d-shader/sm6.h \
 	libs/vkd3d-shader/spirv.c \
 	libs/vkd3d-shader/trace.c \
 	libs/vkd3d-shader/vkd3d_shader.map \
diff --git a/include/vkd3d_shader.h b/include/vkd3d_shader.h
index 7178ac5f..349af41b 100644
--- a/include/vkd3d_shader.h
+++ b/include/vkd3d_shader.h
@@ -596,6 +596,11 @@ enum vkd3d_shader_source_type
      * model 1, 2, and 3 shaders. \since 1.3
      */
     VKD3D_SHADER_SOURCE_D3D_BYTECODE,
+    /**
+     * A 'DirectX Intermediate Language' shader embedded in a DXBC container. This is
+     * the format used for Direct3D shader model 6 shaders. \since 1.6
+     */
+    VKD3D_SHADER_SOURCE_DXBC_DXIL,
 
     VKD3D_FORCE_32_BIT_ENUM(VKD3D_SHADER_SOURCE_TYPE),
 };
diff --git a/libs/vkd3d-shader/dxbc.c b/libs/vkd3d-shader/dxbc.c
index 17be2306..e5d62456 100644
--- a/libs/vkd3d-shader/dxbc.c
+++ b/libs/vkd3d-shader/dxbc.c
@@ -1998,6 +1998,9 @@ static int shdr_handler(const char *data, DWORD data_size, DWORD tag, void *cont
                 return ret;
             break;
 
+        case TAG_DXIL:
+            desc->is_dxil = true;
+            /* fall through */
         case TAG_SHDR:
         case TAG_SHEX:
             if (desc->byte_code)
@@ -2010,10 +2013,6 @@ static int shdr_handler(const char *data, DWORD data_size, DWORD tag, void *cont
             TRACE("Skipping AON9 shader code chunk.\n");
             break;
 
-        case TAG_DXIL:
-            FIXME("Skipping DXIL shader model 6+ code chunk.\n");
-            break;
-
         default:
             TRACE("Skipping chunk %#x.\n", tag);
             break;
@@ -2029,7 +2028,7 @@ void free_shader_desc(struct vkd3d_shader_desc *desc)
     vkd3d_shader_free_shader_signature(&desc->patch_constant_signature);
 }
 
-static int shader_extract_from_dxbc(const void *dxbc, size_t dxbc_length,
+int shader_extract_from_dxbc(const void *dxbc, size_t dxbc_length,
         struct vkd3d_shader_message_context *message_context, const char *source_name, struct vkd3d_shader_desc *desc)
 {
     int ret;
@@ -2074,6 +2073,11 @@ int vkd3d_shader_sm4_parser_create(const struct vkd3d_shader_compile_info *compi
         vkd3d_free(sm4);
         return ret;
     }
+    if (shader_desc->is_dxil)
+    {
+        WARN("Shader is DXBC_DXIL.\n");
+        return VKD3D_ERROR_INVALID_ARGUMENT;
+    }
 
     if (!shader_sm4_init(sm4, shader_desc->byte_code, shader_desc->byte_code_size,
             compile_info->source_name, &shader_desc->output_signature, message_context))
diff --git a/libs/vkd3d-shader/dxil.c b/libs/vkd3d-shader/dxil.c
new file mode 100644
index 00000000..8185ee29
--- /dev/null
+++ b/libs/vkd3d-shader/dxil.c
@@ -0,0 +1,813 @@
+/*
+ * Copyright 2022 Conor McCarthy for CodeWeavers
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, USA
+ */
+
+#include "vkd3d_shader_private.h"
+#include "sm6.h"
+
+struct vkd3d_shader_sm6_record
+{
+    unsigned int code;
+    unsigned int operand_count;
+    uint64_t operands[];
+};
+
+struct vkd3d_shader_sm6_block
+{
+    const struct vkd3d_shader_sm6_block *parent;
+    enum dxil_bc_block_id id;
+    unsigned int abbrev_len;
+    unsigned int start;
+    unsigned int length;
+    unsigned int level;
+
+    /* The abbrev, block and record structs are not relocatable. */
+    struct vkd3d_shader_sm6_abbrev **abbrevs;
+    size_t abbrev_capacity;
+    size_t abbrev_count;
+    unsigned int blockinfo_bid;
+
+    struct vkd3d_shader_sm6_block **child_blocks;
+    size_t child_block_capacity;
+    size_t child_block_count;
+
+    struct vkd3d_shader_sm6_record **records;
+    size_t record_capacity;
+    size_t record_count;
+};
+
+struct vkd3d_shader_sm6_parser
+{
+    const uint32_t *start, *end;
+
+    struct vkd3d_shader_sm6_block root_block;
+    struct vkd3d_shader_sm6_block *current_block;
+
+    struct vkd3d_shader_sm6_global_abbrev **abbrevs;
+    size_t abbrev_capacity;
+    size_t abbrev_count;
+
+    struct vkd3d_shader_parser p;
+};
+
+struct vkd3d_shader_sm6_abbrev_operand
+{
+    uint64_t context;
+    bool (*read_operand)(struct vkd3d_shader_parser *parser, uint64_t context, uint64_t *operand);
+};
+
+struct vkd3d_shader_sm6_abbrev
+{
+    unsigned int count;
+    bool is_array;
+    struct vkd3d_shader_sm6_abbrev_operand operands[];
+};
+
+struct vkd3d_shader_sm6_global_abbrev
+{
+    unsigned int block_id;
+    struct vkd3d_shader_sm6_abbrev abbrev;
+};
+
+static struct vkd3d_shader_sm6_parser *vkd3d_shader_sm6_parser(struct vkd3d_shader_parser *parser)
+{
+    return CONTAINING_RECORD(parser, struct vkd3d_shader_sm6_parser, p);
+}
+
+static void shader_sm6_reset(struct vkd3d_shader_parser *parser)
+{
+    parser->failed = false;
+}
+
+static bool shader_sm6_is_end(struct vkd3d_shader_parser *parser)
+{
+    struct vkd3d_shader_sm6_parser *sm6 = vkd3d_shader_sm6_parser(parser);
+
+    return parser->ptr == sm6->end;
+}
+
+static inline unsigned int shader_sm6_read_uint32(struct vkd3d_shader_parser *parser)
+{
+    if (shader_sm6_is_end(parser))
+    {
+        parser->failed = true;
+        return 0;
+    }
+    return *parser->ptr++;
+}
+
+static inline unsigned int shader_sm6_read_bits(struct vkd3d_shader_parser *parser, unsigned int length)
+{
+    unsigned int l, prev_len = 0;
+    uint32_t bits;
+
+    if (!length)
+        return 0;
+
+    if (shader_sm6_is_end(parser))
+    {
+        parser->failed = true;
+        return 0;
+    }
+
+    bits = *parser->ptr >> parser->bitpos;
+    l = 32 - parser->bitpos;
+    /* The result of uint32 >> 32 is undefined, so bitpos must be <= 31 */
+    if (l <= length)
+    {
+        ++parser->ptr;
+        if (shader_sm6_is_end(parser) && l < length)
+        {
+            parser->failed = true;
+            return bits;
+        }
+        parser->bitpos = 0;
+        bits |= *parser->ptr << l;
+        prev_len = l;
+    }
+    parser->bitpos += length - prev_len;
+
+    return bits & ((1 << length) - 1);
+}
+
+static uint64_t shader_sm6_read_vbr(struct vkd3d_shader_parser *parser, unsigned int length)
+{
+    unsigned int bits, flag, mask, shift = 0;
+    uint64_t result = 0;
+
+    if (!length)
+        return 0;
+
+    if (shader_sm6_is_end(parser))
+    {
+        parser->failed = true;
+        return 0;
+    }
+
+    flag = 1 << (length - 1);
+    mask = flag - 1;
+    do
+    {
+        bits = shader_sm6_read_bits(parser, length);
+        result |= (uint64_t)(bits & mask) << shift;
+        shift += length - 1;
+    } while ((bits & flag) && !parser->failed);
+
+    return result;
+}
+
+static inline void shader_sm6_align_32(struct vkd3d_shader_parser *parser)
+{
+    if (!parser->bitpos)
+        return;
+
+    if (shader_sm6_is_end(parser))
+    {
+        parser->failed = true;
+        return;
+    }
+
+    ++parser->ptr;
+    parser->bitpos = 0;
+}
+
+static bool shader_sm6_record_handle_blockinfo(struct vkd3d_shader_parser *parser,
+        struct vkd3d_shader_sm6_record *record)
+{
+    struct vkd3d_shader_sm6_parser *sm6 = vkd3d_shader_sm6_parser(parser);
+    struct vkd3d_shader_sm6_block *block = sm6->current_block;
+
+    switch (record->code)
+    {
+        case SETBID:
+            if (!record->operand_count)
+                return false;
+            if (record->operands[0] > ~0u)
+                WARN("Truncating block id %"PRIu64".\n", record->operands[0]);
+            block->blockinfo_bid = record->operands[0];
+            break;
+        case BLOCKNAME:
+        case SETRECORDNAME:
+            break;
+        default:
+            FIXME("Unhandled BLOCKINFO record type %u.\n", record->code);
+            break;
+    }
+
+    return true;
+}
+
+static bool shader_sm6_block_add_record(struct vkd3d_shader_sm6_block *block, struct vkd3d_shader_sm6_record *record)
+{
+    unsigned int reserve;
+
+    switch (block->id)
+    {
+        case CONSTANTS_BLOCK: reserve = 32; break;
+        case FUNCTION_BLOCK: reserve = 128; break;
+        case METADATA_BLOCK: reserve = 32; break;
+        case TYPE_BLOCK: reserve = 32; break;
+        default: reserve = 8; break;
+    }
+    reserve = max(reserve, block->record_count + 1);
+    if (!vkd3d_array_reserve((void **)&block->records, &block->record_capacity, reserve, sizeof(*block->records)))
+    {
+        ERR("Failed to allocate %u records.\n", reserve);
+        return false;
+    }
+
+    block->records[block->record_count++] = record;
+
+    return true;
+}
+
+static bool shader_sm6_read_unabbrev_record(struct vkd3d_shader_parser *parser)
+{
+    struct vkd3d_shader_sm6_parser *sm6 = vkd3d_shader_sm6_parser(parser);
+    struct vkd3d_shader_sm6_block *block = sm6->current_block;
+    struct vkd3d_shader_sm6_record *record;
+    unsigned int code, count, i;
+
+    code = shader_sm6_read_vbr(parser, 6);
+
+    count = shader_sm6_read_vbr(parser, 6);
+    if (!(record = vkd3d_malloc(sizeof(*record) + count * sizeof(record->operands[0]))))
+    {
+        ERR("Failed to allocate record with %u operands.\n", count);
+        return false;
+    }
+
+    record->code = code;
+    record->operand_count = count;
+
+    for (i = 0; i < count; ++i)
+        record->operands[i] = shader_sm6_read_vbr(parser, 6);
+
+    if (parser->failed)
+    {
+        vkd3d_free(record);
+        return false;
+    }
+
+    if (!shader_sm6_block_add_record(block, record))
+        return false;
+
+    if (block->id == BLOCKINFO_BLOCK)
+        return shader_sm6_record_handle_blockinfo(parser, record);
+
+    return true;
+}
+
+static bool shader_sm6_read_literal_operand(struct vkd3d_shader_parser *parser, uint64_t context,
+        uint64_t *op)
+{
+    *op = context;
+    return !parser->failed;
+}
+
+static bool shader_sm6_read_fixed_operand(struct vkd3d_shader_parser *parser, uint64_t context,
+        uint64_t *op)
+{
+    *op = shader_sm6_read_bits(parser, context);
+    return !parser->failed;
+}
+
+static bool shader_sm6_read_vbr_operand(struct vkd3d_shader_parser *parser, uint64_t context,
+        uint64_t *op)
+{
+    *op = shader_sm6_read_vbr(parser, context);
+    return !parser->failed;
+}
+
+static bool shader_sm6_read_char6_operand(struct vkd3d_shader_parser *parser, uint64_t context,
+        uint64_t *op)
+{
+    *op = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789._"[shader_sm6_read_bits(parser, 6)];
+    return !parser->failed;
+}
+
+static bool shader_sm6_read_blob_operand(struct vkd3d_shader_parser *parser, uint64_t context,
+        uint64_t *op)
+{
+    int count = shader_sm6_read_vbr(parser, 6);
+    shader_sm6_align_32(parser);
+    for (; count > 0; count -= 4)
+        shader_sm6_read_uint32(parser);
+    FIXME("Unhandled blob operand.\n");
+    return false;
+}
+
+static bool shader_sm6_abbrev_init(struct vkd3d_shader_sm6_abbrev *abbrev,
+        unsigned int count, struct vkd3d_shader_parser *parser)
+{
+    enum dxil_bc_abbrev_type prev_type, type;
+    unsigned int i;
+
+    abbrev->is_array = false;
+
+    for (i = 0, prev_type = 0; i < count && !parser->failed; ++i)
+    {
+        if (shader_sm6_read_bits(parser, 1))
+        {
+            if (prev_type == ABBREV_ARRAY)
+            {
+                FIXME("Unexpected literal abbreviation after array.\n");
+                return false;
+            }
+            abbrev->operands[i].context = shader_sm6_read_vbr(parser, 8);
+            abbrev->operands[i].read_operand = shader_sm6_read_literal_operand;
+            continue;
+        }
+
+        switch (type = shader_sm6_read_bits(parser, 3))
+        {
+            case ABBREV_FIXED:
+            case ABBREV_VBR:
+                abbrev->operands[i].context = shader_sm6_read_vbr(parser, 5);
+                abbrev->operands[i].read_operand = (type == ABBREV_FIXED) ? shader_sm6_read_fixed_operand
+                        : shader_sm6_read_vbr_operand;
+                break;
+
+            case ABBREV_ARRAY:
+                if (prev_type == ABBREV_ARRAY || i != count - 2)
+                {
+                    FIXME("Unexpected array abbreviation.\n");
+                    return false;
+                }
+                abbrev->is_array = true;
+                --i;
+                break;
+
+            case ABBREV_CHAR:
+                abbrev->operands[i].read_operand = shader_sm6_read_char6_operand;
+                break;
+
+            case ABBREV_BLOB:
+                if (prev_type == ABBREV_ARRAY)
+                {
+                    FIXME("Unexpected blob abbreviation after array.\n");
+                    return false;
+                }
+                abbrev->operands[i].read_operand = shader_sm6_read_blob_operand;
+                break;
+        }
+
+        count -= (prev_type == ABBREV_ARRAY);
+        prev_type = type;
+    }
+
+    abbrev->count = count;
+
+    return !parser->failed;
+}
+
+static bool shader_sm6_add_global_abbrev(struct vkd3d_shader_parser *parser)
+{
+    struct vkd3d_shader_sm6_parser *sm6 = vkd3d_shader_sm6_parser(parser);
+    struct vkd3d_shader_sm6_block *block = sm6->current_block;
+    struct vkd3d_shader_sm6_global_abbrev *global_abbrev;
+    unsigned int count = shader_sm6_read_vbr(parser, 5);
+
+    assert(block->id == 0);
+
+    if (!vkd3d_array_reserve((void **)&sm6->abbrevs, &sm6->abbrev_capacity, sm6->abbrev_count + 1, sizeof(*sm6->abbrevs))
+            || !(global_abbrev = vkd3d_malloc(sizeof(*global_abbrev) + count * sizeof(global_abbrev->abbrev.operands[0]))))
+    {
+        ERR("Failed to allocate global abbreviation.\n");
+        return false;
+    }
+
+    if (!shader_sm6_abbrev_init(&global_abbrev->abbrev, count, parser))
+        return false;
+
+    global_abbrev->block_id = block->blockinfo_bid;
+
+    sm6->abbrevs[sm6->abbrev_count++] = global_abbrev;
+
+    return true;
+}
+
+static bool shader_sm6_add_block_abbrev(struct vkd3d_shader_parser *parser)
+{
+    struct vkd3d_shader_sm6_parser *sm6 = vkd3d_shader_sm6_parser(parser);
+    struct vkd3d_shader_sm6_block *block = sm6->current_block;
+    struct vkd3d_shader_sm6_abbrev *abbrev;
+    unsigned int count;
+
+    if (block->id == BLOCKINFO_BLOCK)
+        return shader_sm6_add_global_abbrev(parser);
+
+    count = shader_sm6_read_vbr(parser, 5);
+    if (!vkd3d_array_reserve((void **)&block->abbrevs, &block->abbrev_capacity, block->abbrev_count + 1, sizeof(*block->abbrevs))
+            || !(abbrev = vkd3d_malloc(sizeof(*abbrev) + count * sizeof(abbrev->operands[0]))))
+    {
+        ERR("Failed to allocate block abbreviation.\n");
+        return false;
+    }
+
+    if (!shader_sm6_abbrev_init(abbrev, count, parser))
+        return false;
+
+    block->abbrevs[block->abbrev_count++] = abbrev;
+
+    return true;
+}
+
+static bool shader_sm6_read_abbrev_record(struct vkd3d_shader_parser *parser, unsigned int abbrev_id)
+{
+    struct vkd3d_shader_sm6_parser *sm6 = vkd3d_shader_sm6_parser(parser);
+    struct vkd3d_shader_sm6_block *block = sm6->current_block;
+    struct vkd3d_shader_sm6_record *temp, *record;
+    struct vkd3d_shader_sm6_abbrev *abbrev;
+    unsigned int i, count, array_len;
+    uint64_t code;
+
+    if (abbrev_id >= block->abbrev_count)
+    {
+        FIXME("Invalid abbreviation id %u.\n", abbrev_id);
+        return false;
+    }
+
+    abbrev = block->abbrevs[abbrev_id];
+    if (!(count = abbrev->count))
+        return true;
+    if (count == 1 && abbrev->is_array)
+        return false;
+
+    /* First operand is the record code. The array is included in the count, but will be done separately. */
+    count -= abbrev->is_array + 1;
+    if (!(record = vkd3d_malloc(sizeof(*record) + count * sizeof(record->operands[0]))))
+    {
+        ERR("Failed to allocate record with %u operands.\n", count);
+        return false;
+    }
+
+    if (!abbrev->operands[0].read_operand(parser, abbrev->operands[0].context, &code))
+        return false;
+    if (code > ~0u)
+        FIXME("Invalid 64-bit record code %#"PRIx64".\n", code);
+    record->code = code;
+
+    for (i = 0; i < count; ++i)
+        if (!abbrev->operands[i + 1].read_operand(parser, abbrev->operands[i + 1].context, &record->operands[i]))
+            return false;
+    record->operand_count = count;
+
+    /* An array can occur only as the last operand. */
+    if (abbrev->is_array)
+    {
+        array_len = shader_sm6_read_vbr(parser, 6);
+        if (!(temp = vkd3d_realloc(record, sizeof(*record) + (count + array_len) * sizeof(record->operands[0]))))
+        {
+            ERR("Failed to allocate record with %u operands.\n", count + array_len);
+            vkd3d_free(record);
+            return false;
+        }
+        record = temp;
+
+        for (i = 0; i < array_len; ++i)
+        {
+            if (!abbrev->operands[count + 1].read_operand(parser, abbrev->operands[count + 1].context,
+                    &record->operands[count + i]))
+            {
+                vkd3d_free(record);
+                return false;
+            }
+        }
+        record->operand_count += array_len;
+    }
+
+    if (!shader_sm6_block_add_record(block, record))
+        return false;
+
+    if (block->id == BLOCKINFO_BLOCK)
+        return shader_sm6_record_handle_blockinfo(parser, record);
+
+    return true;
+}
+
+static bool shader_sm6_block_init(struct vkd3d_shader_sm6_block *block, const struct vkd3d_shader_sm6_block *parent,
+        struct vkd3d_shader_parser *parser);
+
+static bool shader_sm6_block_read(struct vkd3d_shader_sm6_block *parent, struct vkd3d_shader_parser *parser)
+{
+    struct vkd3d_shader_sm6_parser *sm6 = vkd3d_shader_sm6_parser(parser);
+    unsigned int reserve = (parent->id == MODULE_BLOCK) ? 12 : 2;
+    struct vkd3d_shader_sm6_block *block;
+
+    sm6->current_block = parent;
+
+    do
+    {
+        unsigned int abbrev_id = shader_sm6_read_bits(parser, parent->abbrev_len);
+
+        switch (abbrev_id)
+        {
+            case END_BLOCK:
+                shader_sm6_align_32(parser);
+                return true;
+
+            case ENTER_SUBBLOCK:
+                if (!(block = vkd3d_malloc(sizeof(*block))) || !vkd3d_array_reserve((void **)&parent->child_blocks,
+                        &parent->child_block_capacity, max(reserve, parent->child_block_count + 1),
+                        sizeof(*parent->child_blocks)))
+                {
+                    ERR("Failed to allocate block.\n");
+                    return false;
+                }
+
+                if (!shader_sm6_block_init(block, parent, parser))
+                    return false;
+
+                parent->child_blocks[parent->child_block_count++] = block;
+                sm6->current_block = parent;
+                break;
+
+            case DEFINE_ABBREV:
+                if (!shader_sm6_add_block_abbrev(parser))
+                    return false;
+                break;
+
+            case UNABBREV_RECORD:
+                if (!shader_sm6_read_unabbrev_record(parser))
+                {
+                    FIXME("Failed to read unabbreviated record.\n");
+                    return false;
+                }
+                break;
+
+            default:
+                if (!shader_sm6_read_abbrev_record(parser, abbrev_id - 4))
+                {
+                    FIXME("Failed to read abbreviated record.\n");
+                    return false;
+                }
+                break;
+        }
+    } while (!parser->failed);
+
+    return false;
+}
+
+static inline unsigned int shader_sm6_compute_global_abbrev_count_for_block_id(struct vkd3d_shader_sm6_parser *sm6,
+        unsigned int block_id)
+{
+    unsigned int i, count;
+
+    for (i = 0, count = 0; i < sm6->abbrev_count; ++i)
+        count += sm6->abbrevs[i]->block_id == block_id;
+
+    return count;
+}
+
+static bool shader_sm6_block_init(struct vkd3d_shader_sm6_block *block, const struct vkd3d_shader_sm6_block *parent,
+        struct vkd3d_shader_parser *parser)
+{
+    struct vkd3d_shader_sm6_parser *sm6 = vkd3d_shader_sm6_parser(parser);
+    unsigned int i, abbrev_count = 0;
+    bool ret;
+
+    block->parent = parent;
+    block->level = parent ? parent->level + 1 : 0;
+    block->id = shader_sm6_read_vbr(parser, 8);
+    block->abbrev_len = shader_sm6_read_vbr(parser, 4);
+    shader_sm6_align_32(parser);
+    block->length = shader_sm6_read_uint32(parser);
+    block->start = parser->ptr - sm6->start;
+    block->abbrevs = NULL;
+    block->abbrev_capacity = 0;
+
+    if (parser->failed)
+        return false;
+
+    if ((block->abbrev_count = shader_sm6_compute_global_abbrev_count_for_block_id(sm6, block->id)))
+    {
+        if (!vkd3d_array_reserve((void **)&block->abbrevs, &block->abbrev_capacity,
+                block->abbrev_count, sizeof(*block->abbrevs)))
+        {
+            ERR("Failed to allocate block abbreviations.\n");
+            return false;
+        }
+
+        for (i = 0; i < sm6->abbrev_count; ++i)
+            if (sm6->abbrevs[i]->block_id == block->id)
+                block->abbrevs[abbrev_count++] = &sm6->abbrevs[i]->abbrev;
+
+        assert(abbrev_count == block->abbrev_count);
+    }
+
+    block->child_blocks = NULL;
+    block->child_block_capacity = 0;
+    block->child_block_count = 0;
+    block->records = NULL;
+    block->record_capacity = 0;
+    block->record_count = 0;
+
+    ret = shader_sm6_block_read(block, parser);
+
+    for (i = abbrev_count; i < block->abbrev_count; ++i)
+        vkd3d_free(block->abbrevs[i]);
+    vkd3d_free(block->abbrevs);
+    block->abbrevs = NULL;
+    block->abbrev_count = 0;
+
+    return ret;
+}
+
+static void shader_sm6_global_abbrevs_cleanup(struct vkd3d_shader_sm6_global_abbrev **abbrevs, unsigned int count)
+{
+    unsigned int i;
+
+    for (i = 0; i < count; ++i)
+        vkd3d_free(abbrevs[i]);
+    vkd3d_free(abbrevs);
+}
+
+static void shader_sm6_block_destroy(struct vkd3d_shader_sm6_block *block)
+{
+    unsigned int i;
+
+    for (i = 0; i < block->record_count; ++i)
+        vkd3d_free(block->records[i]);
+    vkd3d_free(block->records);
+
+    for (i = 0; i < block->child_block_count; ++i)
+    {
+        shader_sm6_block_destroy(block->child_blocks[i]);
+        vkd3d_free(block->child_blocks[i]);
+    }
+    vkd3d_free(block->child_blocks);
+}
+
+static void shader_sm6_destroy(struct vkd3d_shader_parser *parser)
+{
+    struct vkd3d_shader_sm6_parser *sm6 = vkd3d_shader_sm6_parser(parser);
+
+    shader_sm6_block_destroy(&sm6->root_block);
+    free_shader_desc(&parser->shader_desc);
+    vkd3d_free(sm6);
+}
+
+static const struct vkd3d_shader_parser_ops shader_sm6_parser_ops =
+{
+    .parser_reset = shader_sm6_reset,
+    .parser_destroy = shader_sm6_destroy,
+};
+
+static bool shader_sm6_init(struct vkd3d_shader_sm6_parser *sm6, const uint32_t *byte_code,
+        size_t byte_code_size, const char *source_name, const struct vkd3d_shader_signature *output_signature,
+        struct vkd3d_shader_message_context *message_context)
+{
+    uint32_t version_token, dxil_version, token_count;
+    struct vkd3d_shader_sm6_block *block;
+    enum dxil_bc_block_abbreviation abbr;
+    struct vkd3d_shader_version version;
+    unsigned int word_count;
+
+    word_count = byte_code_size / sizeof(*byte_code);
+    if (word_count < 2)
+    {
+        FIXME("Invalid byte code size %zu.\n", byte_code_size);
+        return false;
+    }
+
+    version_token = byte_code[0];
+    TRACE("Compiler version: 0x%08x.\n", version_token);
+    token_count = byte_code[1];
+    TRACE("Token count: %u.\n", token_count);
+
+    if (token_count < 6 || word_count < token_count)
+    {
+        FIXME("Invalid token count %u.\n", token_count);
+        return false;
+    }
+
+    if (byte_code[2] != TAG_DXIL)
+        WARN("Unrecognised magic number 0x%08x.\n", byte_code[2]);
+
+    dxil_version = byte_code[3];
+    TRACE("DXIL version: 0x%08x.\n", dxil_version);
+
+    if (byte_code[4] < 16)
+    {
+        FIXME("Invalid bitcode chunk offset %u.\n", byte_code[4]);
+        return false;
+    }
+    sm6->start = (const uint32_t *)((const char*)&byte_code[2] + byte_code[4]);
+    if (sm6->start[0] != BITCODE_MAGIC)
+        WARN("Unrecognised magic number 0x%08x.\n", sm6->start[0]);
+
+    sm6->end = &sm6->start[(byte_code[5] + sizeof(*sm6->start) - 1) / sizeof(*sm6->start)];
+
+    if ((version.type = version_token >> 16) >= VKD3D_SHADER_TYPE_COUNT)
+        FIXME("Unrecognised shader type %#x.\n", version.type);
+
+    version.major = VKD3D_SM6_VERSION_MAJOR(version_token);
+    version.minor = VKD3D_SM6_VERSION_MINOR(version_token);
+
+    if ((abbr = sm6->start[1] & 3) != ENTER_SUBBLOCK)
+    {
+        FIXME("The first block abbreviation must be ENTER_SUBBLOCK, but is %u.\n", abbr);
+        return false;
+    }
+
+    vkd3d_shader_parser_init(&sm6->p, message_context, source_name, &version, &shader_sm6_parser_ops);
+    sm6->p.ptr = &sm6->start[1];
+    sm6->p.bitpos = 2;
+
+    block = &sm6->root_block;
+    if (!shader_sm6_block_init(block, NULL, &sm6->p))
+        return false;
+    shader_sm6_global_abbrevs_cleanup(sm6->abbrevs, sm6->abbrev_count);
+
+    if (block->start + block->length != sm6->p.ptr - sm6->start)
+    {
+        FIXME("Invalid block end position %zu, expected %u.\n", sm6->p.ptr - sm6->start,
+                block->start + block->length);
+    }
+    if (sm6->p.ptr != sm6->end)
+    {
+        FIXME("Invalid module end position %zu, expected %zu.\n", sm6->p.ptr - sm6->start,
+                sm6->end - sm6->start);
+    }
+
+    shader_sm6_block_destroy(&sm6->root_block);
+    sm6->root_block.records = NULL;
+    sm6->root_block.record_count = 0;
+    sm6->root_block.child_blocks = NULL;
+    sm6->root_block.child_block_count = 0;
+
+    return true;
+}
+
+int vkd3d_shader_sm6_parser_create(const struct vkd3d_shader_compile_info *compile_info,
+        struct vkd3d_shader_message_context *message_context, struct vkd3d_shader_parser **parser)
+{
+    struct vkd3d_shader_desc *shader_desc;
+    struct vkd3d_shader_sm6_parser *sm6;
+    uint32_t *byte_code = NULL;
+    bool succeeded;
+    int ret;
+
+    if (!(sm6 = vkd3d_calloc(1, sizeof(*sm6))))
+    {
+        ERR("Failed to allocate parser.\n");
+        return VKD3D_ERROR_OUT_OF_MEMORY;
+    }
+
+    shader_desc = &sm6->p.shader_desc;
+    if ((ret = shader_extract_from_dxbc(compile_info->source.code, compile_info->source.size,
+            message_context, compile_info->source_name, shader_desc)) < 0)
+    {
+        WARN("Failed to extract shader, vkd3d result %d.\n", ret);
+        vkd3d_free(sm6);
+        return ret;
+    }
+    if (!shader_desc->is_dxil)
+    {
+        WARN("DXIL chunk not found.\n");
+        return VKD3D_ERROR_INVALID_ARGUMENT;
+    }
+
+    sm6->p.shader_desc = *shader_desc;
+    shader_desc = &sm6->p.shader_desc;
+
+    if (((uintptr_t)shader_desc->byte_code & (VKD3D_DXBC_CHUNK_ALIGNMENT - 1)))
+    {
+        /* LLVM bitcode should be 32-bit aligned, but this is not always the case in the DXBC container
+         * due to missing padding after signature names. Get an aligned copy to prevent unaligned access. */
+        if (!(byte_code = vkd3d_malloc(align(shader_desc->byte_code_size, VKD3D_DXBC_CHUNK_ALIGNMENT))))
+            ERR("Failed to allocate aligned chunk. Unaligned access will occur.\n");
+        else
+            memcpy(byte_code, shader_desc->byte_code, shader_desc->byte_code_size);
+    }
+
+    succeeded = shader_sm6_init(sm6, byte_code ? byte_code : shader_desc->byte_code, shader_desc->byte_code_size,
+            compile_info->source_name, &shader_desc->output_signature, message_context);
+    vkd3d_free(byte_code);
+
+    if (!succeeded)
+    {
+        WARN("Failed to initialise shader parser.\n");
+        vkd3d_free(sm6);
+        return VKD3D_ERROR_INVALID_ARGUMENT;
+    }
+
+    *parser = &sm6->p;
+
+    return VKD3D_OK;
+}
diff --git a/libs/vkd3d-shader/sm6.h b/libs/vkd3d-shader/sm6.h
new file mode 100644
index 00000000..47c3ebea
--- /dev/null
+++ b/libs/vkd3d-shader/sm6.h
@@ -0,0 +1,64 @@
+/*
+ * Copyright 2022 Conor McCarthy for CodeWeavers
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, USA
+ */
+
+#ifndef __VKD3D_SM6_H
+#define __VKD3D_SM6_H
+
+#define VKD3D_SM6_VERSION_MAJOR(version) (((version) >> 4) & 0xf)
+#define VKD3D_SM6_VERSION_MINOR(version) (((version) >> 0) & 0xf)
+
+enum dxil_bc_block_id
+{
+    BLOCKINFO_BLOCK       =  0,
+    MODULE_BLOCK          =  8,
+    PARAMATTR_BLOCK       =  9,
+    PARAMATTR_GROUP_BLOCK = 10,
+    CONSTANTS_BLOCK       = 11,
+    FUNCTION_BLOCK        = 12,
+    VALUE_SYMTAB_BLOCK    = 14,
+    METADATA_BLOCK        = 15,
+    METADATA_ATTACHMENT_BLOCK = 16,
+    TYPE_BLOCK            = 17,
+    USELIST_BLOCK         = 18,
+};
+
+enum dxil_bc_blockinfo_code
+{
+    SETBID = 1,
+    BLOCKNAME = 2,
+    SETRECORDNAME = 3,
+};
+
+enum dxil_bc_block_abbreviation
+{
+    END_BLOCK = 0,
+    ENTER_SUBBLOCK = 1,
+    DEFINE_ABBREV = 2,
+    UNABBREV_RECORD = 3,
+};
+
+enum dxil_bc_abbrev_type
+{
+    ABBREV_FIXED = 1,
+    ABBREV_VBR   = 2,
+    ABBREV_ARRAY = 3,
+    ABBREV_CHAR  = 4,
+    ABBREV_BLOB  = 5,
+};
+
+#endif  /* __VKD3D_SM6_H */
diff --git a/libs/vkd3d-shader/trace.c b/libs/vkd3d-shader/trace.c
index 6c30edc9..48dfc136 100644
--- a/libs/vkd3d-shader/trace.c
+++ b/libs/vkd3d-shader/trace.c
@@ -1919,6 +1919,12 @@ enum vkd3d_result vkd3d_dxbc_binary_to_text(struct vkd3d_shader_parser *parser,
             shader_get_type_prefix(shader_version->type), shader_version->major,
             shader_version->minor, compiler.colours.reset);
 
+    if (shader_version->major >= 6)
+    {
+        FIXME("Shader model 6 tracing is not implemented yet.\n");
+        return VKD3D_ERROR_NOT_IMPLEMENTED;
+    }
+
     indent = 0;
     vkd3d_shader_parser_reset(parser);
     while (!vkd3d_shader_parser_is_end(parser))
diff --git a/libs/vkd3d-shader/vkd3d_shader_main.c b/libs/vkd3d-shader/vkd3d_shader_main.c
index 1575a004..2cdb5833 100644
--- a/libs/vkd3d-shader/vkd3d_shader_main.c
+++ b/libs/vkd3d-shader/vkd3d_shader_main.c
@@ -402,6 +402,8 @@ static const char *shader_get_source_type_suffix(enum vkd3d_shader_source_type t
             return "hlsl";
         case VKD3D_SHADER_SOURCE_D3D_BYTECODE:
             return "d3dbc";
+        case VKD3D_SHADER_SOURCE_DXBC_DXIL:
+            return "dxil";
         default:
             FIXME("Unhandled source type %#x.\n", type);
             return "bin";
@@ -1045,6 +1047,13 @@ static int scan_with_parser(const struct vkd3d_shader_compile_info *compile_info
         vkd3d_shader_parser_reset(parser);
     }
 
+    if (parser->shader_version.major >= 6)
+    {
+        FIXME("DXIL scanning is not implemented yet.\n");
+        ret = VKD3D_ERROR_NOT_IMPLEMENTED;
+        goto done;
+    }
+
     while (!vkd3d_shader_parser_is_end(parser))
     {
         vkd3d_shader_parser_read_instruction(parser, &instruction);
@@ -1079,7 +1088,10 @@ static int scan_dxbc(const struct vkd3d_shader_compile_info *compile_info,
     struct vkd3d_shader_parser *parser;
     int ret;
 
-    if ((ret = vkd3d_shader_sm4_parser_create(compile_info, message_context, &parser)) < 0)
+    ret = (compile_info->source_type == VKD3D_SHADER_SOURCE_DXBC_DXIL)
+            ? vkd3d_shader_sm6_parser_create(compile_info, message_context, &parser)
+            : vkd3d_shader_sm4_parser_create(compile_info, message_context, &parser);
+    if (ret < 0)
     {
         WARN("Failed to initialise shader parser.\n");
         return ret;
@@ -1126,6 +1138,7 @@ int vkd3d_shader_scan(const struct vkd3d_shader_compile_info *compile_info, char
 
     switch (compile_info->source_type)
     {
+        case VKD3D_SHADER_SOURCE_DXBC_DXIL:
         case VKD3D_SHADER_SOURCE_DXBC_TPF:
             ret = scan_dxbc(compile_info, &message_context);
             break;
@@ -1283,6 +1296,25 @@ static int compile_d3d_bytecode(const struct vkd3d_shader_compile_info *compile_
     return VKD3D_ERROR;
 }
 
+static int compile_dxbc_dxil(const struct vkd3d_shader_compile_info *compile_info,
+        struct vkd3d_shader_code *out, struct vkd3d_shader_message_context *message_context)
+{
+    struct vkd3d_shader_scan_descriptor_info scan_descriptor_info;
+    struct vkd3d_shader_compile_info scan_info;
+    int ret;
+
+    scan_info = *compile_info;
+    scan_descriptor_info.type = VKD3D_SHADER_STRUCTURE_TYPE_SCAN_DESCRIPTOR_INFO;
+    scan_descriptor_info.next = scan_info.next;
+    scan_info.next = &scan_descriptor_info;
+
+    if ((ret = scan_dxbc(&scan_info, message_context)) < 0)
+        return ret;
+
+    FIXME("DXIL compilation is not implemented yet.\n");
+    return VKD3D_ERROR_NOT_IMPLEMENTED;
+}
+
 int vkd3d_shader_compile(const struct vkd3d_shader_compile_info *compile_info,
         struct vkd3d_shader_code *out, char **messages)
 {
@@ -1313,6 +1345,10 @@ int vkd3d_shader_compile(const struct vkd3d_shader_compile_info *compile_info,
             ret = compile_d3d_bytecode(compile_info, out, &message_context);
             break;
 
+        case VKD3D_SHADER_SOURCE_DXBC_DXIL:
+            ret = compile_dxbc_dxil(compile_info, out, &message_context);
+            break;
+
         default:
             vkd3d_unreachable();
     }
@@ -1470,6 +1506,7 @@ const enum vkd3d_shader_source_type *vkd3d_shader_get_supported_source_types(uns
         VKD3D_SHADER_SOURCE_DXBC_TPF,
         VKD3D_SHADER_SOURCE_HLSL,
         VKD3D_SHADER_SOURCE_D3D_BYTECODE,
+        VKD3D_SHADER_SOURCE_DXBC_DXIL,
     };
 
     TRACE("count %p.\n", count);
@@ -1504,6 +1541,14 @@ const enum vkd3d_shader_target_type *vkd3d_shader_get_supported_target_types(
         VKD3D_SHADER_TARGET_D3D_ASM,
     };
 
+    static const enum vkd3d_shader_target_type dxbc_dxil_types[] =
+    {
+        VKD3D_SHADER_TARGET_SPIRV_BINARY,
+#ifdef HAVE_SPIRV_TOOLS
+        VKD3D_SHADER_TARGET_SPIRV_TEXT,
+#endif
+    };
+
     TRACE("source_type %#x, count %p.\n", source_type, count);
 
     switch (source_type)
@@ -1520,6 +1565,10 @@ const enum vkd3d_shader_target_type *vkd3d_shader_get_supported_target_types(
             *count = ARRAY_SIZE(d3dbc_types);
             return d3dbc_types;
 
+        case VKD3D_SHADER_SOURCE_DXBC_DXIL:
+            *count = ARRAY_SIZE(dxbc_dxil_types);
+            return dxbc_dxil_types;
+
         default:
             *count = 0;
             return NULL;
diff --git a/libs/vkd3d-shader/vkd3d_shader_private.h b/libs/vkd3d-shader/vkd3d_shader_private.h
index aca5606b..66cee9bb 100644
--- a/libs/vkd3d-shader/vkd3d_shader_private.h
+++ b/libs/vkd3d-shader/vkd3d_shader_private.h
@@ -770,6 +770,7 @@ struct vkd3d_shader_desc
 {
     const uint32_t *byte_code;
     size_t byte_code_size;
+    bool is_dxil;
     struct vkd3d_shader_signature input_signature;
     struct vkd3d_shader_signature output_signature;
     struct vkd3d_shader_signature patch_constant_signature;
@@ -934,6 +935,7 @@ struct vkd3d_shader_parser
     struct vkd3d_shader_desc shader_desc;
     struct vkd3d_shader_version shader_version;
     const uint32_t *ptr;
+    unsigned int bitpos;
     const struct vkd3d_shader_parser_ops *ops;
 };
 
@@ -1071,11 +1073,15 @@ int vkd3d_shader_sm1_parser_create(const struct vkd3d_shader_compile_info *compi
         struct vkd3d_shader_message_context *message_context, struct vkd3d_shader_parser **parser);
 int vkd3d_shader_sm4_parser_create(const struct vkd3d_shader_compile_info *compile_info,
         struct vkd3d_shader_message_context *message_context, struct vkd3d_shader_parser **parser);
+int vkd3d_shader_sm6_parser_create(const struct vkd3d_shader_compile_info *compile_info,
+        struct vkd3d_shader_message_context *message_context, struct vkd3d_shader_parser **parser);
 
 void free_shader_desc(struct vkd3d_shader_desc *desc);
 
 int shader_parse_input_signature(const void *dxbc, size_t dxbc_length,
         struct vkd3d_shader_message_context *message_context, struct vkd3d_shader_signature *signature);
+int shader_extract_from_dxbc(const void *dxbc, size_t dxbc_length,
+        struct vkd3d_shader_message_context *message_context, const char *source_name, struct vkd3d_shader_desc *desc);
 
 struct vkd3d_glsl_generator;
 
@@ -1239,6 +1245,7 @@ static inline void *vkd3d_find_struct_(const struct vkd3d_struct *chain,
 
 #define VKD3D_DXBC_MAX_SOURCE_COUNT 6
 #define VKD3D_DXBC_HEADER_SIZE (8 * sizeof(uint32_t))
+#define VKD3D_DXBC_CHUNK_ALIGNMENT sizeof(uint32_t)
 
 #define TAG_AON9 VKD3D_MAKE_TAG('A', 'o', 'n', '9')
 #define TAG_DXBC VKD3D_MAKE_TAG('D', 'X', 'B', 'C')
@@ -1257,6 +1264,8 @@ static inline void *vkd3d_find_struct_(const struct vkd3d_struct *chain,
 #define TAG_SHEX VKD3D_MAKE_TAG('S', 'H', 'E', 'X')
 #define TAG_TEXT VKD3D_MAKE_TAG('T', 'E', 'X', 'T')
 
+#define BITCODE_MAGIC 0xdec04342
+
 struct dxbc_writer_section
 {
     uint32_t tag;
-- 
GitLab


https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/44

[PATCH 2/2] vkd3d-compiler: Introduce a dxbc-dxil source type.

[Conor McCarthy]

From: Conor McCarthy <cmccarthy(a)codeweavers.com>

---
 programs/vkd3d-compiler/main.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/programs/vkd3d-compiler/main.c b/programs/vkd3d-compiler/main.c
index 731b1d45..ea5ea989 100644
--- a/programs/vkd3d-compiler/main.c
+++ b/programs/vkd3d-compiler/main.c
@@ -73,6 +73,10 @@ source_type_info[] =
         "d3dbc",        "Legacy Direct3D byte-code.\n"
         "                This is the format used for Direct3D shader model 1, 2, and 3 shaders.\n",
         true, VKD3D_SHADER_TARGET_SPIRV_BINARY},
+    {VKD3D_SHADER_SOURCE_DXBC_DXIL,
+        "dxbc-dxil",    "A 'DirectX Intermediate Language' shader embedded in a DXBC container.\n"
+        "                This is the format used for Direct3D shader model 6 shaders.\n",
+        true, VKD3D_SHADER_TARGET_SPIRV_BINARY},
 };
 
 static const struct target_type_info
-- 
GitLab

https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/44

[PATCH 2/2] vkd3d-compiler: Introduce a dxbc-dxil source type.

[Marvin]

Hi,

It looks like your patch introduced the new failures shown below.
Please investigate and fix them before resubmitting your patch.
If they are not new, fixing them anyway would help a lot. Otherwise
please ask for the known failures list to be updated.

The full results can be found at:
https://testbot.winehq.org/JobDetails.pl?Key=125596

Your paranoid android.


=== debian11 (build log) ===

Task: Patch failed to apply

=== debian11b (build log) ===

Task: Patch failed to apply

[Giovanni Mascellani (＠giomasce)]

Giovanni Mascellani (@giomasce) commented about libs/vkd3d-shader/dxil.c:
> +{
> +    unsigned int l, prev_len = 0;
> +    uint32_t bits;
> +
> +    if (!length)
> +        return 0;
> +
> +    if (shader_sm6_is_end(parser))
> +    {
> +        parser->failed = true;
> +        return 0;
> +    }
> +
> +    bits = *parser->ptr >> parser->bitpos;
> +    l = 32 - parser->bitpos;
> +    /* The result of uint32 >> 32 is undefined, so bitpos must be <= 31 */
> I would convert this to an assertion. It gives the same information, but can be checked by the compiler.

-- 
https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/44#note_12817

[Giovanni Mascellani (＠giomasce)]

Giovanni Mascellani (@giomasce) commented about libs/vkd3d-shader/dxbc.c:
> return ret;
>              break;
>
> +        case TAG_DXIL:
> +            desc->is_dxil = true;
> +            /* fall through */

It's mostly a theoretical thing, but if a shader has both a `DXIL` and a `SHDR` section it might end up with `is_dxil` set, but `byte_code` pointing to the TPF code. I would just write something like `desc->is_dxil = tag == TAG_DXIL;`.

-- 
https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/44#note_12816

[Giovanni Mascellani (＠giomasce)]

Giovanni Mascellani (@giomasce) commented about libs/vkd3d-shader/dxil.c:
> +static inline unsigned int shader_sm6_read_uint32(struct vkd3d_shader_parser *parser)
> +{
> +    if (shader_sm6_is_end(parser))
> +    {
> +        parser->failed = true;
> +        return 0;
> +    }
> +    return *parser->ptr++;
> +}
> +
> +static inline unsigned int shader_sm6_read_bits(struct vkd3d_shader_parser *parser, unsigned int length)
> +{
> +    unsigned int l, prev_len = 0;
> +    uint32_t bits;
> +
> +    if (!length)
> Mostly for the sake of documentation, I would add an assertion that `length < 32`.

-- 
https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/44#note_12818

[Giovanni Mascellani (＠giomasce)]

Giovanni Mascellani (@giomasce) commented about libs/vkd3d-shader/dxil.c:
> +    return CONTAINING_RECORD(parser, struct vkd3d_shader_sm6_parser, p);
> +}
> +
> +static void shader_sm6_reset(struct vkd3d_shader_parser *parser)
> +{
> +    parser->failed = false;
> +}
> +
> +static bool shader_sm6_is_end(struct vkd3d_shader_parser *parser)
> +{
> +    struct vkd3d_shader_sm6_parser *sm6 = vkd3d_shader_sm6_parser(parser);
> +
> +    return parser->ptr == sm6->end;
> +}
> +
> +static inline unsigned int shader_sm6_read_uint32(struct vkd3d_shader_parser *parser)
> Here and in other places it would feel more natural to use `uint32_t` instead of `unsigned int`, even if they end up being the same thing on all architectures we care about. You sometimes use fixed-width types and sometimes don't. Is there a logic for that? Since I guess that DXIL/LLVM specifications mandate the bit width for basically everything, I'd say that it makes sense to always use fixed-width integer types.

-- 
https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/44#note_12819

[Giovanni Mascellani (＠giomasce)]

Giovanni Mascellani (@giomasce) commented about libs/vkd3d-shader/dxil.c:
> +
> +    if (!length)
> +        return 0;
> +
> +    if (shader_sm6_is_end(parser))
> +    {
> +        parser->failed = true;
> +        return 0;
> +    }
> +
> +    flag = 1 << (length - 1);
> +    mask = flag - 1;
> +    do
> +    {
> +        bits = shader_sm6_read_bits(parser, length);
> +        result |= (uint64_t)(bits & mask) << shift;
> We should probably check when `shift` becomes 64 or larger and produce an error if it does. We shouldn't invoke undefined behavior even on invalid input streams.

-- 
https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/44#note_12820

[Giovanni Mascellani (＠giomasce)]

I'm not done with the review yet, but let me share the first batch of comments I  got while reading your patches. Hopefully I'll do more tomorrow.

-- 
https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/44#note_12821

[Henri Verbeet (＠hverbeet)]

> From patch 1/2:
> ```diff
> @@ -243,6 +244,7 @@ libvkd3d_shader_la_SOURCES = \
>   libs/vkd3d-shader/hlsl_sm4.c \
>   libs/vkd3d-shader/preproc.h \
>   libs/vkd3d-shader/sm4.h \
> +	libs/vkd3d-shader/sm6.h \
>   libs/vkd3d-shader/spirv.c \
>   libs/vkd3d-shader/trace.c \
>   libs/vkd3d-shader/vkd3d_shader.map \
> ```
> sm6.h is only used by dxil.c. Perhaps that will change in future commits, but until it does, its contents simply belong in dxil.c.
> ```diff
> @@ -2074,6 +2073,11 @@ int vkd3d_shader_sm4_parser_create(const struct vkd3d_shader_compile_info *compi
>          vkd3d_free(sm4);
>          return ret;
>      }
> +    if (shader_desc->is_dxil)
> +    {
> +        WARN("Shader is DXBC_DXIL.\n");
> +        return VKD3D_ERROR_INVALID_ARGUMENT;
> +    }
>
> if (!shader_sm4_init(sm4, shader_desc->byte_code, shader_desc->byte_code_size,
>              compile_info->source_name, &shader_desc->output_signature, message_context))
>
> ```

I'd argue we shouldn't necessarily reject shaders containing DXIL chunks in vkd3d_shader_sm4_parser_create(), as long as they contain a SHDR/SHEX chunk. That does imply shdr_handler() can't work as-is. We could e.g. initialise "desc->is_dxil" before calling parse_dxbc() in shader_extract_from_dxbc(), and then accept either DXIL or SHDR/SHEX chunks in shdr_handler() depending on whether "is_dxil" is set. Alternatively, we could just use separate handlers for SHDR/SHEX and DXIL, and avoid the "is_dxil" field in struct vkd3d_shader_desc.
> ```diff
> +static inline unsigned int shader_sm6_read_uint32(struct vkd3d_shader_parser *parser)
> +{
> +    if (shader_sm6_is_end(parser))
> +    {
> +        parser->failed = true;
> +        return 0;
> +    }
> +    return *parser->ptr++;
> +}
>
> ```

- "inline" is probably superfluous above.
  - This should just return an uint32_t.
That applies to e.g. shader_sm6_read_bits(), as well.
> ```diff
> +static bool shader_sm6_record_handle_blockinfo(struct vkd3d_shader_parser *parser,
> +        struct vkd3d_shader_sm6_record *record)
> +{
> +    struct vkd3d_shader_sm6_parser *sm6 = vkd3d_shader_sm6_parser(parser);
> +    struct vkd3d_shader_sm6_block *block = sm6->current_block;
>  ...
> +}
> ```

This is called shader_sm6_record_handle_blockinfo(), but it actually operates on a block. There's also no reason to pass the parser as argument here.
> ```diff
> +static bool shader_sm6_block_read(struct vkd3d_shader_sm6_block *parent, struct vkd3d_shader_parser *parser)
> +{
> +    struct vkd3d_shader_sm6_parser *sm6 = vkd3d_shader_sm6_parser(parser);
> +    unsigned int reserve = (parent->id == MODULE_BLOCK) ? 12 : 2;
> +    struct vkd3d_shader_sm6_block *block;
> +
> +    sm6->current_block = parent;
> +
> +    do
> +    {
> +        unsigned int abbrev_id = shader_sm6_read_bits(parser, parent->abbrev_len);
> +
> +        switch (abbrev_id)
> +        {
> +            case END_BLOCK:
> +                shader_sm6_align_32(parser);
> +                return true;
> +
> +            case ENTER_SUBBLOCK:
> +                if (!(block = vkd3d_malloc(sizeof(*block))) || !vkd3d_array_reserve((void **)&parent->child_blocks,
> +                        &parent->child_block_capacity, max(reserve, parent->child_block_count + 1),
> +                        sizeof(*parent->child_blocks)))
> +                {
> +                    ERR("Failed to allocate block.\n");
> +                    return false;
> +                }
> +
> +                if (!shader_sm6_block_init(block, parent, parser))
> +                    return false;
> +
> +                parent->child_blocks[parent->child_block_count++] = block;
> +                sm6->current_block = parent;
> +                break;
> +
> +            case DEFINE_ABBREV:
> +                if (!shader_sm6_add_block_abbrev(parser))
> +                    return false;
> +                break;
> +
> +            case UNABBREV_RECORD:
> +                if (!shader_sm6_read_unabbrev_record(parser))
> +                {
> +                    FIXME("Failed to read unabbreviated record.\n");
> +                    return false;
> +                }
> +                break;
> +
> +            default:
> +                if (!shader_sm6_read_abbrev_record(parser, abbrev_id - 4))
> +                {
> +                    FIXME("Failed to read abbreviated record.\n");
> +                    return false;
> +                }
> +                break;
> +        }
> +    } while (!parser->failed);
> +
> +    return false;
> +}
> ```

This seems like a nice place to split things; we could just error out on unrecognised abbreviation IDs, and then introduce the code to handle them one by one. That would allow a bunch of code for handling abbreviations and abbreviated records to just be deferred to later patches, and allow us to focus on the basic structure of the DXIL frontend in the initial patches.
> ```diff
> @@ -1504,6 +1541,14 @@ const enum vkd3d_shader_target_type *vkd3d_shader_get_supported_target_types(
>          VKD3D_SHADER_TARGET_D3D_ASM,
>      };
>
> +    static const enum vkd3d_shader_target_type dxbc_dxil_types[] =
> +    {
> +        VKD3D_SHADER_TARGET_SPIRV_BINARY,
> +#ifdef HAVE_SPIRV_TOOLS
> +        VKD3D_SHADER_TARGET_SPIRV_TEXT,
> +#endif
> +    };
> +
>      TRACE("source_type %#x, count %p.\n", source_type, count);
>
> switch (source_type)
> ```

I imagine it will be a while until we can actually generate SPIR-V. The first step should probably be getting the disassembler; that would mean VKD3D_SHADER_TARGET_D3D_ASM here, or perhaps more likely a new VKD3D_SHADER_TARGET_DXIL_ASM.
> ```diff
> @@ -1257,6 +1264,8 @@ static inline void *vkd3d_find_struct_(const struct vkd3d_struct *chain,
>  #define TAG_SHEX VKD3D_MAKE_TAG('S', 'H', 'E', 'X')
>  #define TAG_TEXT VKD3D_MAKE_TAG('T', 'E', 'X', 'T')
>
> +#define BITCODE_MAGIC 0xdec04342
> +
> ```

That's "VKD3D_MAKE_TAG('B', 'C', 0xc0, 0xde)", right? Also, I don't think there should ever be a reason for this to be visible outside of dxil.c.

About patch 2/2:

It doesn't necessarily need to be addressed in this patch, but note that the autodetection code (i.e., the "if (options.source_type == VKD3D_SHADER_SOURCE_NONE)" block in main()) currently defaults to VKD3D_SHADER_SOURCE_DXBC_TPF for DXBC inputs. It might be nice to detect DXIL there as well. That probably doesn't require a full DXBC parser; iterating over the chunks in a DXBC isn't too complicated.

-- 
https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/44#note_12831

[Zebediah Figura (＠zfigura)]

> sm6.h is only used by dxil.c. Perhaps that will change in future commits, but until it does, its contents simply belong in dxil.c.

There's a good chance we'll want to emit sm6 from the HLSL compiler. Why not be proactive, and avoid the code churn?

-- 
https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/44#note_12840