Block records are not processed; only the bitcode is validated.
-- v2: vkd3d-shader/dxil: Read global abbreviated operands. vkd3d-shader/dxil: Read local abbreviated operands. vkd3d-compiler: Introduce a dxbc-dxil source type. vkd3d-shader/dxil: Read and validate DXIL bitcode unabbreviated blocks.
From: Conor McCarthy cmccarthy@codeweavers.com
--- Makefile.am | 2 + include/vkd3d_shader.h | 9 + libs/vkd3d-shader/dxbc.c | 19 +- libs/vkd3d-shader/dxil.c | 536 +++++++++++++++++++++++ libs/vkd3d-shader/trace.c | 6 + libs/vkd3d-shader/vkd3d_shader_main.c | 48 +- libs/vkd3d-shader/vkd3d_shader_private.h | 7 + 7 files changed, 621 insertions(+), 6 deletions(-) create mode 100644 libs/vkd3d-shader/dxil.c
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..79355c6f 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), }; @@ -632,6 +637,10 @@ enum vkd3d_shader_target_type * An 'OpenGL Shading Language' shader. \since 1.3 */ VKD3D_SHADER_TARGET_GLSL, + /** + * DirectX Intermediate Language shader assembly. \since 1.6 + */ + VKD3D_SHADER_TARGET_DXIL_ASM,
VKD3D_FORCE_32_BIT_ENUM(VKD3D_SHADER_TARGET_TYPE), }; diff --git a/libs/vkd3d-shader/dxbc.c b/libs/vkd3d-shader/dxbc.c index 17be2306..351cf64a 100644 --- a/libs/vkd3d-shader/dxbc.c +++ b/libs/vkd3d-shader/dxbc.c @@ -1998,8 +1998,20 @@ static int shdr_handler(const char *data, DWORD data_size, DWORD tag, void *cont return ret; break;
+ case TAG_DXIL: + if (!desc->is_dxil) + { + WARN("Skipping DXIL chunk.\n"); + break; + } + /* fall through */ case TAG_SHDR: case TAG_SHEX: + if (tag != TAG_DXIL && desc->is_dxil) + { + WARN("Skipping TPF chunk.\n"); + break; + } if (desc->byte_code) FIXME("Multiple shader code chunks.\n"); desc->byte_code = (const uint32_t *)data; @@ -2010,10 +2022,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 +2037,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; @@ -2067,6 +2075,7 @@ int vkd3d_shader_sm4_parser_create(const struct vkd3d_shader_compile_info *compi }
shader_desc = &sm4->p.shader_desc; + shader_desc->is_dxil = false; if ((ret = shader_extract_from_dxbc(compile_info->source.code, compile_info->source.size, message_context, compile_info->source_name, shader_desc)) < 0) { diff --git a/libs/vkd3d-shader/dxil.c b/libs/vkd3d-shader/dxil.c new file mode 100644 index 00000000..4b01102b --- /dev/null +++ b/libs/vkd3d-shader/dxil.c @@ -0,0 +1,536 @@ +/* + * 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" + +#define VKD3D_SM6_VERSION_MAJOR(version) (((version) >> 4) & 0xf) +#define VKD3D_SM6_VERSION_MINOR(version) (((version) >> 0) & 0xf) + +#define BITCODE_MAGIC VKD3D_MAKE_TAG('B', 'C', 0xc0, 0xde) + +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, +}; + +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; + + 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_parser p; +}; + +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 uint32_t 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 uint32_t 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; + + assert(length < 32); + + if (shader_sm6_is_end(parser)) + { + parser->failed = true; + return 0; + } + + assert(parser->bitpos < 32); + bits = *parser->ptr >> parser->bitpos; + l = 32 - parser->bitpos; + 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_block_handle_blockinfo_record(struct vkd3d_shader_sm6_block *block, + struct vkd3d_shader_sm6_record *record) +{ + 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; + + if (block->id == BLOCKINFO_BLOCK) + return shader_sm6_block_handle_blockinfo_record(block, 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; + + 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: + FIXME("Unhandled abbreviation definition.\n"); + return false; + + case UNABBREV_RECORD: + if (!shader_sm6_read_unabbrev_record(parser)) + { + FIXME("Failed to read unabbreviated record.\n"); + return false; + } + break; + + default: + FIXME("Unhandled abbreviated record %u.\n", abbrev_id); + return false; + } + } while (!parser->failed); + + return false; +} + +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); + 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; + + if (parser->failed) + return false; + + 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); + + return ret; +} + +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; + + 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; + shader_desc->is_dxil = true; + 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; + } + + 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/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..524aeb1d 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,11 @@ 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_DXIL_ASM, + }; + TRACE("source_type %#x, count %p.\n", source_type, count);
switch (source_type) @@ -1520,6 +1562,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..5b1c2c8f 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')
From: Conor McCarthy cmccarthy@codeweavers.com
--- programs/vkd3d-compiler/main.c | 7 +++++++ 1 file changed, 7 insertions(+)
diff --git a/programs/vkd3d-compiler/main.c b/programs/vkd3d-compiler/main.c index 731b1d45..495b0db7 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 @@ -94,6 +98,9 @@ target_type_info[] = {VKD3D_SHADER_TARGET_D3D_ASM, "d3d-asm", "A shader in Direct3D assembly form.\n", false}, + {VKD3D_SHADER_TARGET_DXIL_ASM, + "dxil-asm", "A shader in DirectX Intermediate Language assembly form.\n", + false}, {VKD3D_SHADER_TARGET_D3D_BYTECODE, "d3dbc", "Legacy Direct3D byte-code.\n" " This is the format used for Direct3D shader model 1, 2, and 3 shaders.\n",
From: Conor McCarthy cmccarthy@codeweavers.com
--- libs/vkd3d-shader/dxil.c | 251 ++++++++++++++++++++++++++++++++++++++- 1 file changed, 247 insertions(+), 4 deletions(-)
diff --git a/libs/vkd3d-shader/dxil.c b/libs/vkd3d-shader/dxil.c index 4b01102b..0621fca7 100644 --- a/libs/vkd3d-shader/dxil.c +++ b/libs/vkd3d-shader/dxil.c @@ -53,6 +53,15 @@ enum dxil_bc_block_abbreviation UNABBREV_RECORD = 3, };
+enum dxil_bc_abbrev_type +{ + ABBREV_FIXED = 1, + ABBREV_VBR = 2, + ABBREV_ARRAY = 3, + ABBREV_CHAR = 4, + ABBREV_BLOB = 5, +}; + struct vkd3d_shader_sm6_record { unsigned int code; @@ -69,6 +78,10 @@ struct vkd3d_shader_sm6_block 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; @@ -90,6 +103,19 @@ struct vkd3d_shader_sm6_parser 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[]; +}; + 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); @@ -278,6 +304,208 @@ static bool shader_sm6_read_unabbrev_record(struct vkd3d_shader_parser *parser) 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_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) + { + FIXME("Unhandled global abbreviation.\n"); + return false; + } + + 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; + + 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);
@@ -316,8 +544,9 @@ static bool shader_sm6_block_read(struct vkd3d_shader_sm6_block *parent, struct break;
case DEFINE_ABBREV: - FIXME("Unhandled abbreviation definition.\n"); - return false; + if (!shader_sm6_add_block_abbrev(parser)) + return false; + break;
case UNABBREV_RECORD: if (!shader_sm6_read_unabbrev_record(parser)) @@ -328,8 +557,12 @@ static bool shader_sm6_block_read(struct vkd3d_shader_sm6_block *parent, struct break;
default: - FIXME("Unhandled abbreviated record %u.\n", abbrev_id); - return false; + if (!shader_sm6_read_abbrev_record(parser, abbrev_id - 4)) + { + FIXME("Failed to read abbreviated record.\n"); + return false; + } + break; } } while (!parser->failed);
@@ -340,6 +573,7 @@ static bool shader_sm6_block_init(struct vkd3d_shader_sm6_block *block, const st struct vkd3d_shader_parser *parser) { struct vkd3d_shader_sm6_parser *sm6 = vkd3d_shader_sm6_parser(parser); + unsigned int i; bool ret;
block->parent = parent; @@ -349,6 +583,9 @@ static bool shader_sm6_block_init(struct vkd3d_shader_sm6_block *block, const st 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; + block->abbrev_count = 0;
if (parser->failed) return false; @@ -362,6 +599,12 @@ static bool shader_sm6_block_init(struct vkd3d_shader_sm6_block *block, const st
ret = shader_sm6_block_read(block, parser);
+ for (i = 0; i < block->abbrev_count; ++i) + vkd3d_free(block->abbrevs[i]); + vkd3d_free(block->abbrevs); + block->abbrevs = NULL; + block->abbrev_count = 0; + return ret; }
From: Conor McCarthy cmccarthy@codeweavers.com
--- libs/vkd3d-shader/dxil.c | 83 ++++++++++++++++++++++++++++++++++++---- 1 file changed, 76 insertions(+), 7 deletions(-)
diff --git a/libs/vkd3d-shader/dxil.c b/libs/vkd3d-shader/dxil.c index 0621fca7..69457ca6 100644 --- a/libs/vkd3d-shader/dxil.c +++ b/libs/vkd3d-shader/dxil.c @@ -100,6 +100,10 @@ struct vkd3d_shader_sm6_parser 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; };
@@ -116,6 +120,12 @@ struct vkd3d_shader_sm6_abbrev 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); @@ -407,6 +417,32 @@ static bool shader_sm6_abbrev_init(struct vkd3d_shader_sm6_abbrev *abbrev, 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); @@ -415,10 +451,7 @@ static bool shader_sm6_add_block_abbrev(struct vkd3d_shader_parser *parser) unsigned int count;
if (block->id == BLOCKINFO_BLOCK) - { - FIXME("Unhandled global abbreviation.\n"); - return false; - } + 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)) @@ -569,11 +602,22 @@ static bool shader_sm6_block_read(struct vkd3d_shader_sm6_block *parent, struct 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; + unsigned int i, abbrev_count = 0; bool ret;
block->parent = parent; @@ -585,11 +629,26 @@ static bool shader_sm6_block_init(struct vkd3d_shader_sm6_block *block, const st block->start = parser->ptr - sm6->start; block->abbrevs = NULL; block->abbrev_capacity = 0; - block->abbrev_count = 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; @@ -599,7 +658,7 @@ static bool shader_sm6_block_init(struct vkd3d_shader_sm6_block *block, const st
ret = shader_sm6_block_read(block, parser);
- for (i = 0; i < block->abbrev_count; ++i) + for (i = abbrev_count; i < block->abbrev_count; ++i) vkd3d_free(block->abbrevs[i]); vkd3d_free(block->abbrevs); block->abbrevs = NULL; @@ -608,6 +667,15 @@ static bool shader_sm6_block_init(struct vkd3d_shader_sm6_block *block, const st 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; @@ -703,6 +771,7 @@ static bool shader_sm6_init(struct vkd3d_shader_sm6_parser *sm6, const uint32_t 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) {
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=125697
Your paranoid android.
=== debian11 (build log) ===
Task: Patch failed to apply
=== debian11b (build log) ===
Task: Patch failed to apply
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?
The practical answer to that question is "to make the life of your reviewers easier and have your patches go in smoothly". I.e., you're essentially asking a reviewer to take a guess at potential future uses for these, or look at later patches in the series. This could be mitigated by writing a good commit message, of course, but even in that case you may end up in a discussion about future usage which would otherwise not be a concern for the patch in question.
To that point, it's not clear to me that we'd want a sm6.h for outputting sm6 from the HLSL compiler either; I think the code for that should just go into dxil.c as well.
Giovanni Mascellani (@giomasce) commented about libs/vkd3d-shader/dxbc.c:
break;
case TAG_DXIL:if (!desc->is_dxil){WARN("Skipping DXIL chunk.\n");break;}/* fall through */ case TAG_SHDR: case TAG_SHEX:if (tag != TAG_DXIL && desc->is_dxil){WARN("Skipping TPF chunk.\n");break;}
This feels more complicated than it should be. Maybe just `if (tag == TAG_DXIL != desc->is_dxil) { WARN("Skipping code chunk.\n"); break; }`. I am not even sure we really want a `WARN` here.
Giovanni Mascellani (@giomasce) commented about Makefile.am:
libs/vkd3d-shader/hlsl_sm4.c \ libs/vkd3d-shader/preproc.h \ libs/vkd3d-shader/sm4.h \
- libs/vkd3d-shader/sm6.h \
This looks like a leftover.
Giovanni Mascellani (@giomasce) commented about programs/vkd3d-compiler/main.c:
"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},
We probably want to indicate `VKD3D_SHADER_TARGET_DXIL_ASM` as default target.
Giovanni Mascellani (@giomasce) commented about libs/vkd3d-shader/dxil.c:
+#define VKD3D_SM6_VERSION_MAJOR(version) (((version) >> 4) & 0xf) +#define VKD3D_SM6_VERSION_MINOR(version) (((version) >> 0) & 0xf)
+#define BITCODE_MAGIC VKD3D_MAKE_TAG('B', 'C', 0xc0, 0xde)
+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,
I don't know why, but on https://www.llvm.org/docs/BitCodeFormat.html#metadata-attachment it seems that the name is just `METADATA_ATTACHMENT`, without `_BLOCK`.
Giovanni Mascellani (@giomasce) commented about libs/vkd3d-shader/dxil.c:
- }
- 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;
Are there specific reasons for these magic numbers? It seems they're just used to initialize the capacity, so they're basically irrelevant except perhaps for a little performance optimization. But you seem to have a rather precise idea of what they should be, so I wonder where they come from (maybe I am missing something).
The same applies to `shader_sm6_block_read`.
Giovanni Mascellani (@giomasce) commented about libs/vkd3d-shader/dxil.c:
- 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,
If `vkd3d_array_reserve()` or `shader_sm6_block_init()` fail, then `block` is not freed.
(if you call `vkd3d_array_reserve()` before `vkd3d_malloc()` then you don't have to care about that path, because `vkd3d_array_reserve()` doesn't leave dangling pointers around)
Giovanni Mascellani (@giomasce) commented about libs/vkd3d-shader/dxil.c:
return shader_sm6_block_handle_blockinfo_record(block, 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]))))
`record` is leaked if `shader_sm6_block_add_record()` fails when calling `vkd3d_array_reserve()`.
Giovanni Mascellani (@giomasce) commented about libs/vkd3d-shader/dxil.c:
+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)
- {
FIXME("Unhandled global abbreviation.\n");return false;- }
- 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]))))
`abbrev` is leaked if `shader_sm6_abbrev_init()` fails.
Giovanni Mascellani (@giomasce) commented about libs/vkd3d-shader/dxil.c:
- 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]))))
`record` is leaked in a number of failure paths.
Giovanni Mascellani (@giomasce) commented about libs/vkd3d-shader/dxil.c:
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]))))
Here too `global_abbrev` is leaked on failure paths.
Giovanni Mascellani (@giomasce) commented about libs/vkd3d-shader/dxil.c:
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);
Instead of this, couldn't you add `count--` to the `case ABBREV_ARRAY` block? It feels easier to understand to me.
Giovanni Mascellani (@giomasce) commented about libs/vkd3d-shader/dxil.c:
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:
Similarly to the array case, you could check that a blob only appears as the last item of an abbreviation.
Giovanni Mascellani (@giomasce) commented about libs/vkd3d-shader/dxil.c:
- 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)
Not a strong opinion, but I would find it more idiomatic to use `UINT_MAX` instead of `~0u`.
Giovanni Mascellani (@giomasce) commented about libs/vkd3d-shader/dxil.c:
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);
Here it makes sense to use the well-known `BLOCKINFO_BLOCK` name.
We probably want to indicate `VKD3D_SHADER_TARGET_DXIL_ASM` as default target.
No, I think VKD3D_SHADER_TARGET_SPIRV_BINARY is right. It's not going to work right now, but I think it's the correct default in the long term.
No, I think VKD3D_SHADER_TARGET_SPIRV_BINARY is right. It's not going to work right now, but I think it's the correct default in the long term.
I guessed we might want to switch it to SPIR-V once that flag is legal. Anyway, it's not that I care too much.
On Thu Nov 3 12:38:57 2022 +0000, Giovanni Mascellani wrote:
No, I think VKD3D_SHADER_TARGET_SPIRV_BINARY is right. It's not going
to work right now, but I think it's the correct default in the long term. I guessed we might want to switch it to SPIR-V once that flag is legal. Anyway, it's not that I care too much.
Personally I think changing the default in a tool like this is a bad idea. (In fact, I'd argue that it should be held to a similar standard of API stability as the library itself.)
Personally I think changing the default in a tool like this is a bad idea. (In fact, I'd argue that it should be held to a similar standard of API stability as the library itself.)
Right, that was my line of thinking as well.
On Thu Nov 3 12:17:01 2022 +0000, Giovanni Mascellani wrote:
I don't know why, but on https://www.llvm.org/docs/BitCodeFormat.html#metadata-attachment it seems that the name is just `METADATA_ATTACHMENT`, without `_BLOCK`.
Yes it's probably an oversight on their part. I changed it for clarity and consistency in our code.
-
Conor McCarthy -
Conor McCarthy (＠cmccarthy)
-
Giovanni Mascellani (＠giomasce)
-
Henri Verbeet (＠hverbeet)
-
Marvin -
Zebediah Figura (＠zfigura)