From patch 1/2:
@@ -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.
@@ -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.
+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.
+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.
+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.
@@ -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.
@@ -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.