[PATCH v14 0/6] MR471: vkd3d-shader/dxil: Implement the DXIL GEP and LOAD instructions.
-- v14: vkd3d-shader/dxil: Implement the DXIL LOAD instruction. vkd3d-shader/dxil: Implement the DXIL GEP instruction. https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/471
From: Conor McCarthy <cmccarthy(a)codeweavers.com> --- libs/vkd3d-shader/vkd3d_shader_private.h | 1 - 1 file changed, 1 deletion(-) diff --git a/libs/vkd3d-shader/vkd3d_shader_private.h b/libs/vkd3d-shader/vkd3d_shader_private.h index bff494539..cfd47dd37 100644 --- a/libs/vkd3d-shader/vkd3d_shader_private.h +++ b/libs/vkd3d-shader/vkd3d_shader_private.h @@ -799,7 +799,6 @@ struct vkd3d_shader_immediate_constant_buffer struct vkd3d_shader_indexable_temp { - struct list entry; unsigned int register_idx; unsigned int register_size; unsigned int component_count; -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/471
From: Conor McCarthy <cmccarthy(a)codeweavers.com> --- libs/vkd3d-shader/d3d_asm.c | 2 + libs/vkd3d-shader/dxil.c | 189 ++++++++++++++++++++++- libs/vkd3d-shader/tpf.c | 2 + libs/vkd3d-shader/vkd3d_shader_private.h | 2 + 4 files changed, 194 insertions(+), 1 deletion(-) diff --git a/libs/vkd3d-shader/d3d_asm.c b/libs/vkd3d-shader/d3d_asm.c index 442c1e414..f6d511e7c 100644 --- a/libs/vkd3d-shader/d3d_asm.c +++ b/libs/vkd3d-shader/d3d_asm.c @@ -1679,6 +1679,8 @@ static void shader_dump_instruction(struct vkd3d_d3d_asm_compiler *compiler, ins->declaration.indexable_temp.register_idx, compiler->colours.reset); shader_print_subscript(compiler, ins->declaration.indexable_temp.register_size, NULL); shader_print_uint_literal(compiler, ", ", ins->declaration.indexable_temp.component_count, ""); + if (ins->declaration.indexable_temp.alignment) + shader_print_uint_literal(compiler, ", align ", ins->declaration.indexable_temp.alignment, ""); break; case VKD3DSIH_DCL_INPUT_PS: diff --git a/libs/vkd3d-shader/dxil.c b/libs/vkd3d-shader/dxil.c index ca7e0e723..bb23aa868 100644 --- a/libs/vkd3d-shader/dxil.c +++ b/libs/vkd3d-shader/dxil.c @@ -25,6 +25,10 @@ #define BITCODE_MAGIC VKD3D_MAKE_TAG('B', 'C', 0xc0, 0xde) #define DXIL_OP_MAX_OPERANDS 17 +static const uint64_t MAX_ALIGNMENT_EXPONENT = 29; +static const uint64_t GLOBALVAR_FLAG_IS_CONSTANT = 1; +static const uint64_t GLOBALVAR_FLAG_EXPLICIT_TYPE = 2; +static const unsigned int GLOBALVAR_ADDRESS_SPACE_SHIFT = 2; static const unsigned int SHADER_DESCRIPTOR_TYPE_COUNT = 4; static const unsigned int dx_max_thread_group_size[3] = {1024, 1024, 64}; @@ -157,6 +161,13 @@ enum bitcode_value_symtab_code VST_CODE_BBENTRY = 2, }; +enum bitcode_linkage +{ + LINKAGE_EXTERNAL = 0, + LINKAGE_APPENDING = 2, + LINKAGE_INTERNAL = 3, +}; + enum dxil_component_type { COMPONENT_TYPE_INVALID = 0, @@ -567,6 +578,8 @@ struct sm6_parser size_t descriptor_capacity; size_t descriptor_count; + unsigned int indexable_temp_count; + struct sm6_value *values; size_t value_count; size_t value_capacity; @@ -2555,6 +2568,17 @@ static enum vkd3d_result sm6_parser_constants_init(struct sm6_parser *sm6, const return VKD3D_OK; } +static bool bitcode_parse_alignment(uint64_t encoded_alignment, unsigned int *alignment) +{ + if (encoded_alignment > MAX_ALIGNMENT_EXPONENT + 1) + { + *alignment = 0; + return false; + } + *alignment = (1u << encoded_alignment) >> 1; + return true; +} + static struct vkd3d_shader_instruction *sm6_parser_require_space(struct sm6_parser *sm6, size_t extra) { if (!shader_instruction_array_reserve(&sm6->p.instructions, sm6->p.instructions.count + extra)) @@ -2576,6 +2600,168 @@ static struct vkd3d_shader_instruction *sm6_parser_add_instruction(struct sm6_pa return ins; } +static void sm6_parser_declare_indexable_temp(struct sm6_parser *sm6, const struct sm6_type *elem_type, + unsigned int count, unsigned int alignment, struct sm6_value *dst) +{ + enum vkd3d_data_type data_type = vkd3d_data_type_from_sm6_type(elem_type); + struct vkd3d_shader_instruction *ins; + + ins = sm6_parser_add_instruction(sm6, VKD3DSIH_DCL_INDEXABLE_TEMP); + ins->declaration.indexable_temp.register_idx = sm6->indexable_temp_count++; + ins->declaration.indexable_temp.register_size = count; + ins->declaration.indexable_temp.alignment = alignment; + ins->declaration.indexable_temp.data_type = data_type; + ins->declaration.indexable_temp.component_count = 1; + + register_init_with_id(&dst->u.reg, VKD3DSPR_IDXTEMP, data_type, ins->declaration.indexable_temp.register_idx); +} + +static bool sm6_parser_declare_global(struct sm6_parser *sm6, const struct dxil_record *record) +{ + const struct sm6_type *type, *scalar_type; + unsigned int alignment, count; + uint64_t address_space, init; + struct sm6_value *dst; + bool is_constant; + + if (!dxil_record_validate_operand_min_count(record, 6, sm6)) + return false; + + if (!(type = sm6_parser_get_type(sm6, record->operands[0]))) + return false; + if (sm6_type_is_array(type)) + { + if (!sm6_type_is_scalar(type->u.array.elem_type)) + { + FIXME("Unsupported nested type class %u.\n", type->u.array.elem_type->class); + vkd3d_shader_parser_error(&sm6->p, VKD3D_SHADER_ERROR_DXIL_INVALID_OPERAND, + "Global array variables with nested type class %u are not supported.", + type->u.array.elem_type->class); + return false; + } + count = type->u.array.count; + scalar_type = type->u.array.elem_type; + } + else if (sm6_type_is_scalar(type)) + { + count = 1; + scalar_type = type; + } + else + { + FIXME("Unsupported type class %u.\n", type->class); + vkd3d_shader_parser_error(&sm6->p, VKD3D_SHADER_ERROR_DXIL_INVALID_OPERAND, + "Global variables of type class %u are not supported.", type->class); + return false; + } + + is_constant = record->operands[1] & GLOBALVAR_FLAG_IS_CONSTANT; + + if (record->operands[1] & GLOBALVAR_FLAG_EXPLICIT_TYPE) + { + address_space = record->operands[1] >> GLOBALVAR_ADDRESS_SPACE_SHIFT; + + if (!(type = sm6_type_get_pointer_to_type(type, address_space, sm6))) + { + WARN("Failed to get pointer type for type class %u, address space %"PRIu64".\n", + type->class, address_space); + vkd3d_shader_parser_error(&sm6->p, VKD3D_SHADER_ERROR_DXIL_INVALID_MODULE, + "Module does not define a pointer type for a global variable."); + return false; + } + } + else + { + if (!sm6_type_is_pointer(type)) + { + WARN("Type is not a pointer.\n"); + vkd3d_shader_parser_error(&sm6->p, VKD3D_SHADER_ERROR_DXIL_INVALID_OPERAND, + "The type of a global variable is not a pointer."); + return false; + } + address_space = type->u.pointer.addr_space; + } + + if ((init = record->operands[2])) + { + if (init - 1 >= sm6->value_capacity) + { + WARN("Invalid value index %"PRIu64" for initialiser.", init - 1); + vkd3d_shader_parser_error(&sm6->p, VKD3D_SHADER_ERROR_DXIL_INVALID_OPERAND, + "Global variable initialiser value index %"PRIu64" is invalid.", init - 1); + return false; + } + } + + /* LINKAGE_EXTERNAL is common but not relevant here. */ + if (record->operands[3] != LINKAGE_EXTERNAL && record->operands[3] != LINKAGE_INTERNAL) + WARN("Ignoring linkage %"PRIu64".\n", record->operands[3]); + + if (!bitcode_parse_alignment(record->operands[4], &alignment)) + WARN("Invalid alignment %"PRIu64".\n", record->operands[4]); + + if (record->operands[5]) + WARN("Ignoring section code %"PRIu64".\n", record->operands[5]); + + if (!sm6_parser_get_global_symbol_name(sm6, sm6->value_count)) + WARN("Missing symbol name for global variable at index %zu.\n", sm6->value_count); + /* TODO: store global symbol names in struct vkd3d_shader_desc? */ + + if (record->operand_count > 6 && record->operands[6]) + WARN("Ignoring visibility %"PRIu64".\n", record->operands[6]); + if (record->operand_count > 7 && record->operands[7]) + WARN("Ignoring thread local mode %"PRIu64".\n", record->operands[7]); + /* record->operands[8] contains unnamed_addr, a flag indicating the address + * is not important, only the content is. This info is not relevant. */ + if (record->operand_count > 9 && record->operands[9]) + WARN("Ignoring external_init %"PRIu64".\n", record->operands[9]); + if (record->operand_count > 10 && record->operands[10]) + WARN("Ignoring dll storage class %"PRIu64".\n", record->operands[10]); + if (record->operand_count > 11 && record->operands[11]) + WARN("Ignoring comdat %"PRIu64".\n", record->operands[11]); + + dst = sm6_parser_get_current_value(sm6); + dst->type = type; + dst->value_type = VALUE_TYPE_REG; + + if (init) + { + FIXME("Unsupported initialiser.\n"); + vkd3d_shader_parser_error(&sm6->p, VKD3D_SHADER_ERROR_DXIL_INVALID_OPERAND, + "Global variable initialisers are not supported."); + return false; + } + else if (is_constant) + { + WARN("Constant array has no initialiser.\n"); + vkd3d_shader_parser_error(&sm6->p, VKD3D_SHADER_ERROR_DXIL_INVALID_OPERAND, + "A constant global variable has no initialiser."); + return false; + } + + if (address_space == ADDRESS_SPACE_DEFAULT) + { + sm6_parser_declare_indexable_temp(sm6, scalar_type, count, alignment, dst); + } + else if (address_space == ADDRESS_SPACE_GROUPSHARED) + { + FIXME("Unsupported TGSM.\n"); + vkd3d_shader_parser_error(&sm6->p, VKD3D_SHADER_ERROR_DXIL_INVALID_OPERAND, + "TGSM global variables are not supported."); + return false; + } + else + { + FIXME("Unhandled address space %"PRIu64".\n", address_space); + vkd3d_shader_parser_error(&sm6->p, VKD3D_SHADER_ERROR_DXIL_INVALID_OPERAND, + "Global variables with address space %"PRIu64" are not supported.", address_space); + return false; + } + + ++sm6->value_count; + return true; +} + static enum vkd3d_result sm6_parser_globals_init(struct sm6_parser *sm6) { const struct dxil_block *block = &sm6->root_block; @@ -2603,7 +2789,8 @@ static enum vkd3d_result sm6_parser_globals_init(struct sm6_parser *sm6) break; case MODULE_CODE_GLOBALVAR: - FIXME("Global variables are not implemented yet.\n"); + if (!sm6_parser_declare_global(sm6, record)) + return VKD3D_ERROR_INVALID_SHADER; break; case MODULE_CODE_VERSION: diff --git a/libs/vkd3d-shader/tpf.c b/libs/vkd3d-shader/tpf.c index 594438a26..61d14e30d 100644 --- a/libs/vkd3d-shader/tpf.c +++ b/libs/vkd3d-shader/tpf.c @@ -1113,6 +1113,8 @@ static void shader_sm4_read_dcl_indexable_temp(struct vkd3d_shader_instruction * { ins->declaration.indexable_temp.register_idx = *tokens++; ins->declaration.indexable_temp.register_size = *tokens++; + ins->declaration.indexable_temp.alignment = 0; + ins->declaration.indexable_temp.data_type = VKD3D_DATA_FLOAT; ins->declaration.indexable_temp.component_count = *tokens; } diff --git a/libs/vkd3d-shader/vkd3d_shader_private.h b/libs/vkd3d-shader/vkd3d_shader_private.h index cfd47dd37..d3d6f77f2 100644 --- a/libs/vkd3d-shader/vkd3d_shader_private.h +++ b/libs/vkd3d-shader/vkd3d_shader_private.h @@ -801,6 +801,8 @@ struct vkd3d_shader_indexable_temp { unsigned int register_idx; unsigned int register_size; + unsigned int alignment; + enum vkd3d_data_type data_type; unsigned int component_count; }; -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/471
From: Conor McCarthy <cmccarthy(a)codeweavers.com> --- libs/vkd3d-shader/dxil.c | 14 ++++++++------ libs/vkd3d-shader/vkd3d_shader_main.c | 8 ++++---- libs/vkd3d-shader/vkd3d_shader_private.h | 2 +- 3 files changed, 13 insertions(+), 11 deletions(-) diff --git a/libs/vkd3d-shader/dxil.c b/libs/vkd3d-shader/dxil.c index bb23aa868..1929fcb48 100644 --- a/libs/vkd3d-shader/dxil.c +++ b/libs/vkd3d-shader/dxil.c @@ -404,6 +404,7 @@ enum sm6_value_type { VALUE_TYPE_FUNCTION, VALUE_TYPE_REG, + VALUE_TYPE_ICB, VALUE_TYPE_HANDLE, }; @@ -429,6 +430,7 @@ struct sm6_value { struct sm6_function_data function; struct vkd3d_shader_register reg; + const struct vkd3d_shader_immediate_constant_buffer *icb; struct sm6_handle_data handle; } u; }; @@ -1900,7 +1902,7 @@ static inline bool sm6_value_is_undef(const struct sm6_value *value) static bool sm6_value_is_icb(const struct sm6_value *value) { - return sm6_value_is_register(value) && value->u.reg.type == VKD3DSPR_IMMCONSTBUFFER; + return value->value_type == VALUE_TYPE_ICB; } static inline unsigned int sm6_value_get_constant_uint(const struct sm6_value *value) @@ -2360,7 +2362,7 @@ static inline double bitcast_uint64_to_double(uint64_t value) return u.double_value; } -static enum vkd3d_result register_allocate_constant_array(struct vkd3d_shader_register *reg, const struct sm6_type *type, +static enum vkd3d_result value_allocate_constant_array(struct sm6_value *dst, const struct sm6_type *type, const uint64_t *operands, struct sm6_parser *sm6) { struct vkd3d_shader_immediate_constant_buffer *icb; @@ -2395,7 +2397,7 @@ static enum vkd3d_result register_allocate_constant_array(struct vkd3d_shader_re "Out of memory allocating an immediate constant buffer of count %u.", count); return VKD3D_ERROR_OUT_OF_MEMORY; } - if ((reg->idx[0].offset = shader_instruction_array_add_icb(&sm6->p.instructions, icb)) == UINT_MAX) + if (!shader_instruction_array_add_icb(&sm6->p.instructions, icb)) { ERR("Failed to store icb object.\n"); vkd3d_free(icb); @@ -2404,8 +2406,8 @@ static enum vkd3d_result register_allocate_constant_array(struct vkd3d_shader_re return VKD3D_ERROR_OUT_OF_MEMORY; } - reg->type = VKD3DSPR_IMMCONSTBUFFER; - reg->idx_count = 1; + dst->value_type = VALUE_TYPE_ICB; + dst->u.icb = icb; icb->data_type = vkd3d_data_type_from_sm6_type(elem_type); icb->element_count = type->u.array.count; @@ -2544,7 +2546,7 @@ static enum vkd3d_result sm6_parser_constants_init(struct sm6_parser *sm6, const if (!dxil_record_validate_operand_count(record, type->u.array.count, type->u.array.count, sm6)) return VKD3D_ERROR_INVALID_SHADER; - if ((ret = register_allocate_constant_array(&dst->u.reg, type, record->operands, sm6)) < 0) + if ((ret = value_allocate_constant_array(dst, type, record->operands, sm6)) < 0) return ret; break; diff --git a/libs/vkd3d-shader/vkd3d_shader_main.c b/libs/vkd3d-shader/vkd3d_shader_main.c index 48af5d976..86e418a68 100644 --- a/libs/vkd3d-shader/vkd3d_shader_main.c +++ b/libs/vkd3d-shader/vkd3d_shader_main.c @@ -2022,14 +2022,14 @@ bool shader_instruction_array_reserve(struct vkd3d_shader_instruction_array *ins return true; } -unsigned int shader_instruction_array_add_icb(struct vkd3d_shader_instruction_array *instructions, +bool shader_instruction_array_add_icb(struct vkd3d_shader_instruction_array *instructions, struct vkd3d_shader_immediate_constant_buffer *icb) { if (!vkd3d_array_reserve((void **)&instructions->icbs, &instructions->icb_capacity, instructions->icb_count + 1, sizeof(*instructions->icbs))) - return UINT_MAX; - instructions->icbs[instructions->icb_count] = icb; - return instructions->icb_count++; + return false; + instructions->icbs[instructions->icb_count++] = icb; + return true; } static struct vkd3d_shader_src_param *shader_instruction_array_clone_src_params( diff --git a/libs/vkd3d-shader/vkd3d_shader_private.h b/libs/vkd3d-shader/vkd3d_shader_private.h index d3d6f77f2..f8b53718a 100644 --- a/libs/vkd3d-shader/vkd3d_shader_private.h +++ b/libs/vkd3d-shader/vkd3d_shader_private.h @@ -1204,7 +1204,7 @@ struct vkd3d_shader_instruction_array bool shader_instruction_array_init(struct vkd3d_shader_instruction_array *instructions, unsigned int reserve); bool shader_instruction_array_reserve(struct vkd3d_shader_instruction_array *instructions, unsigned int reserve); -unsigned int shader_instruction_array_add_icb(struct vkd3d_shader_instruction_array *instructions, +bool shader_instruction_array_add_icb(struct vkd3d_shader_instruction_array *instructions, struct vkd3d_shader_immediate_constant_buffer *icb); bool shader_instruction_array_clone_instruction(struct vkd3d_shader_instruction_array *instructions, unsigned int dst, unsigned int src); -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/471
From: Conor McCarthy <cmccarthy(a)codeweavers.com> --- libs/vkd3d-shader/dxil.c | 47 +++++++++++++++++++----- libs/vkd3d-shader/vkd3d_shader_private.h | 1 + 2 files changed, 38 insertions(+), 10 deletions(-) diff --git a/libs/vkd3d-shader/dxil.c b/libs/vkd3d-shader/dxil.c index 1929fcb48..5990b6df5 100644 --- a/libs/vkd3d-shader/dxil.c +++ b/libs/vkd3d-shader/dxil.c @@ -2603,7 +2603,7 @@ static struct vkd3d_shader_instruction *sm6_parser_add_instruction(struct sm6_pa } static void sm6_parser_declare_indexable_temp(struct sm6_parser *sm6, const struct sm6_type *elem_type, - unsigned int count, unsigned int alignment, struct sm6_value *dst) + unsigned int count, unsigned int alignment, unsigned int init, struct sm6_value *dst) { enum vkd3d_data_type data_type = vkd3d_data_type_from_sm6_type(elem_type); struct vkd3d_shader_instruction *ins; @@ -2614,6 +2614,8 @@ static void sm6_parser_declare_indexable_temp(struct sm6_parser *sm6, const stru ins->declaration.indexable_temp.alignment = alignment; ins->declaration.indexable_temp.data_type = data_type; ins->declaration.indexable_temp.component_count = 1; + /* The initialiser value index will be resolved later so forward references can be handled. */ + ins->declaration.indexable_temp.initialiser = (void *)(uintptr_t)init; register_init_with_id(&dst->u.reg, VKD3DSPR_IDXTEMP, data_type, ins->declaration.indexable_temp.register_idx); } @@ -2726,14 +2728,7 @@ static bool sm6_parser_declare_global(struct sm6_parser *sm6, const struct dxil_ dst->type = type; dst->value_type = VALUE_TYPE_REG; - if (init) - { - FIXME("Unsupported initialiser.\n"); - vkd3d_shader_parser_error(&sm6->p, VKD3D_SHADER_ERROR_DXIL_INVALID_OPERAND, - "Global variable initialisers are not supported."); - return false; - } - else if (is_constant) + if (is_constant && !init) { WARN("Constant array has no initialiser.\n"); vkd3d_shader_parser_error(&sm6->p, VKD3D_SHADER_ERROR_DXIL_INVALID_OPERAND, @@ -2743,7 +2738,7 @@ static bool sm6_parser_declare_global(struct sm6_parser *sm6, const struct dxil_ if (address_space == ADDRESS_SPACE_DEFAULT) { - sm6_parser_declare_indexable_temp(sm6, scalar_type, count, alignment, dst); + sm6_parser_declare_indexable_temp(sm6, scalar_type, count, alignment, init, dst); } else if (address_space == ADDRESS_SPACE_GROUPSHARED) { @@ -2764,9 +2759,30 @@ static bool sm6_parser_declare_global(struct sm6_parser *sm6, const struct dxil_ return true; } +static const struct vkd3d_shader_immediate_constant_buffer *resolve_forward_initialiser( + size_t index, struct sm6_parser *sm6) +{ + const struct sm6_value *value; + + assert(index); + --index; + if (!(value = sm6_parser_get_value_safe(sm6, index)) || !sm6_value_is_icb(value)) + { + WARN("Invalid initialiser index %zu.\n", index); + vkd3d_shader_parser_error(&sm6->p, VKD3D_SHADER_ERROR_DXIL_INVALID_OPERAND, + "Global variable initialiser value index %zu is invalid.", index); + return NULL; + } + else + { + return value->u.icb; + } +} + static enum vkd3d_result sm6_parser_globals_init(struct sm6_parser *sm6) { const struct dxil_block *block = &sm6->root_block; + struct vkd3d_shader_instruction *ins; const struct dxil_record *record; enum vkd3d_result ret; uint64_t version; @@ -2819,6 +2835,17 @@ static enum vkd3d_result sm6_parser_globals_init(struct sm6_parser *sm6) return ret; } + /* Resolve initialiser forward references. */ + for (i = 0; i < sm6->p.instructions.count; ++i) + { + ins = &sm6->p.instructions.elements[i]; + if (ins->handler_idx == VKD3DSIH_DCL_INDEXABLE_TEMP && ins->declaration.indexable_temp.initialiser) + { + ins->declaration.indexable_temp.initialiser = resolve_forward_initialiser( + (uintptr_t)ins->declaration.indexable_temp.initialiser, sm6); + } + } + return VKD3D_OK; } diff --git a/libs/vkd3d-shader/vkd3d_shader_private.h b/libs/vkd3d-shader/vkd3d_shader_private.h index f8b53718a..9980827f9 100644 --- a/libs/vkd3d-shader/vkd3d_shader_private.h +++ b/libs/vkd3d-shader/vkd3d_shader_private.h @@ -804,6 +804,7 @@ struct vkd3d_shader_indexable_temp unsigned int alignment; enum vkd3d_data_type data_type; unsigned int component_count; + const struct vkd3d_shader_immediate_constant_buffer *initialiser; }; struct vkd3d_shader_register_index -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/471
From: Conor McCarthy <cmccarthy(a)codeweavers.com> --- libs/vkd3d-shader/dxil.c | 115 +++++++++++++++++++++++ libs/vkd3d-shader/ir.c | 3 + libs/vkd3d-shader/vkd3d_shader_private.h | 2 + 3 files changed, 120 insertions(+) diff --git a/libs/vkd3d-shader/dxil.c b/libs/vkd3d-shader/dxil.c index 5990b6df5..4249e7f52 100644 --- a/libs/vkd3d-shader/dxil.c +++ b/libs/vkd3d-shader/dxil.c @@ -2195,6 +2195,18 @@ static bool sm6_value_validate_is_handle(const struct sm6_value *value, struct s return true; } +static bool sm6_value_validate_is_pointer(const struct sm6_value *value, struct sm6_parser *sm6) +{ + if (!sm6_type_is_pointer(value->type)) + { + WARN("Operand result type class %u is not a pointer.\n", value->type->class); + vkd3d_shader_parser_error(&sm6->p, VKD3D_SHADER_ERROR_DXIL_INVALID_OPERAND, + "A pointer operand passed to a DXIL instruction is not a pointer."); + return false; + } + return true; +} + static bool sm6_value_validate_is_bool(const struct sm6_value *value, struct sm6_parser *sm6) { const struct sm6_type *type = value->type; @@ -3877,6 +3889,106 @@ static void sm6_parser_emit_extractval(struct sm6_parser *sm6, const struct dxil instruction_dst_param_init_ssa_scalar(ins, sm6); } +static void sm6_parser_emit_gep(struct sm6_parser *sm6, const struct dxil_record *record, + struct vkd3d_shader_instruction *ins, struct sm6_value *dst) +{ + const struct sm6_type *type, *pointee_type; + unsigned int elem_idx, operand_idx = 2; + enum bitcode_address_space addr_space; + const struct sm6_value *elem_value; + struct vkd3d_shader_register *reg; + const struct sm6_value *src; + bool is_in_bounds; + + if (!dxil_record_validate_operand_min_count(record, 5, sm6) + || !(type = sm6_parser_get_type(sm6, record->operands[1])) + || !(src = sm6_parser_get_value_by_ref(sm6, record, NULL, &operand_idx)) + || !sm6_value_validate_is_register(src, sm6) + || !sm6_value_validate_is_pointer(src, sm6) + || !dxil_record_validate_operand_min_count(record, operand_idx + 2, sm6)) + { + return; + } + + if (src->u.reg.idx_count > 1) + { + WARN("Unsupported stacked GEP.\n"); + vkd3d_shader_parser_error(&sm6->p, VKD3D_SHADER_ERROR_DXIL_INVALID_OPERAND, + "A GEP instruction on the result of a previous GEP is unsupported."); + return; + } + + is_in_bounds = record->operands[0]; + + if ((pointee_type = src->type->u.pointer.type) != type) + { + WARN("Type mismatch, type %u width %u vs type %u width %u.\n", type->class, + type->u.width, pointee_type->class, pointee_type->u.width); + vkd3d_shader_parser_warning(&sm6->p, VKD3D_SHADER_WARNING_DXIL_TYPE_MISMATCH, + "Type mismatch in GEP operation arguments."); + } + addr_space = src->type->u.pointer.addr_space; + + if (!(elem_value = sm6_parser_get_value_by_ref(sm6, record, NULL, &operand_idx))) + return; + + /* The first index is always zero, to form a simple pointer dereference. */ + if (sm6_value_get_constant_uint(elem_value)) + { + WARN("Expected constant zero.\n"); + vkd3d_shader_parser_error(&sm6->p, VKD3D_SHADER_ERROR_DXIL_INVALID_OPERAND, + "The pointer dereference index for a GEP instruction is not constant zero."); + return; + } + + if (!sm6_type_is_array(pointee_type)) + { + WARN("Invalid GEP on type class %u.\n", pointee_type->class); + vkd3d_shader_parser_error(&sm6->p, VKD3D_SHADER_ERROR_DXIL_INVALID_OPERAND, + "Source type for index 1 of a GEP instruction is not an array."); + return; + } + + if (!(elem_value = sm6_parser_get_value_by_ref(sm6, record, NULL, &operand_idx))) + return; + + /* If indexing is dynamic, just get the type at offset zero. */ + elem_idx = sm6_value_is_constant(elem_value) ? sm6_value_get_constant_uint(elem_value) : 0; + type = sm6_type_get_element_type_at_index(pointee_type, elem_idx); + if (!type) + { + WARN("Invalid element index %u.\n", elem_idx); + vkd3d_shader_parser_error(&sm6->p, VKD3D_SHADER_ERROR_DXIL_INVALID_OPERAND, + "Element index %u for a GEP instruction is out of bounds.", elem_idx); + return; + } + + if (operand_idx < record->operand_count) + { + FIXME("Multiple element indices are not implemented.\n"); + vkd3d_shader_parser_error(&sm6->p, VKD3D_SHADER_ERROR_DXIL_INVALID_OPERAND, + "Multi-dimensional addressing in GEP instructions is not supported."); + return; + } + + if (!(dst->type = sm6_type_get_pointer_to_type(type, addr_space, sm6))) + { + WARN("Failed to get pointer type for type %u.\n", type->class); + vkd3d_shader_parser_error(&sm6->p, VKD3D_SHADER_ERROR_DXIL_INVALID_MODULE, + "Module does not define a pointer type for a GEP instruction."); + return; + } + + reg = &dst->u.reg; + *reg = src->u.reg; + reg->idx[1].offset = 0; + register_index_address_init(®->idx[1], elem_value, sm6); + reg->idx[1].is_in_bounds = is_in_bounds; + reg->idx_count = 2; + + ins->handler_idx = VKD3DSIH_NOP; +} + static void sm6_parser_emit_ret(struct sm6_parser *sm6, const struct dxil_record *record, struct sm6_block *code_block, struct vkd3d_shader_instruction *ins) { @@ -4079,6 +4191,9 @@ static enum vkd3d_result sm6_parser_function_init(struct sm6_parser *sm6, const case FUNC_CODE_INST_EXTRACTVAL: sm6_parser_emit_extractval(sm6, record, ins, dst); break; + case FUNC_CODE_INST_GEP: + sm6_parser_emit_gep(sm6, record, ins, dst); + break; case FUNC_CODE_INST_RET: sm6_parser_emit_ret(sm6, record, code_block, ins); is_terminator = true; diff --git a/libs/vkd3d-shader/ir.c b/libs/vkd3d-shader/ir.c index 2a3343994..ae1610483 100644 --- a/libs/vkd3d-shader/ir.c +++ b/libs/vkd3d-shader/ir.c @@ -295,10 +295,13 @@ void vsir_register_init(struct vkd3d_shader_register *reg, enum vkd3d_shader_reg reg->data_type = data_type; reg->idx[0].offset = ~0u; reg->idx[0].rel_addr = NULL; + reg->idx[0].is_in_bounds = false; reg->idx[1].offset = ~0u; reg->idx[1].rel_addr = NULL; + reg->idx[1].is_in_bounds = false; reg->idx[2].offset = ~0u; reg->idx[2].rel_addr = NULL; + reg->idx[2].is_in_bounds = false; reg->idx_count = idx_count; reg->dimension = VSIR_DIMENSION_SCALAR; } diff --git a/libs/vkd3d-shader/vkd3d_shader_private.h b/libs/vkd3d-shader/vkd3d_shader_private.h index 9980827f9..68eef0b8d 100644 --- a/libs/vkd3d-shader/vkd3d_shader_private.h +++ b/libs/vkd3d-shader/vkd3d_shader_private.h @@ -811,6 +811,8 @@ struct vkd3d_shader_register_index { const struct vkd3d_shader_src_param *rel_addr; unsigned int offset; + /* address is known to fall within the object (for optimisation) */ + bool is_in_bounds; }; struct vkd3d_shader_register -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/471
From: Conor McCarthy <cmccarthy(a)codeweavers.com> --- libs/vkd3d-shader/dxil.c | 66 +++++++++++++++++++ libs/vkd3d-shader/ir.c | 1 + libs/vkd3d-shader/vkd3d_shader_private.h | 2 + tests/hlsl/matrix-indexing.shader_test | 6 +- tests/hlsl/non-const-indexing.shader_test | 46 ++++++------- .../hlsl/vector-indexing-uniform.shader_test | 4 +- 6 files changed, 97 insertions(+), 28 deletions(-) diff --git a/libs/vkd3d-shader/dxil.c b/libs/vkd3d-shader/dxil.c index 4249e7f52..1709212fa 100644 --- a/libs/vkd3d-shader/dxil.c +++ b/libs/vkd3d-shader/dxil.c @@ -2207,6 +2207,18 @@ static bool sm6_value_validate_is_pointer(const struct sm6_value *value, struct return true; } +static bool sm6_value_validate_is_numeric(const struct sm6_value *value, struct sm6_parser *sm6) +{ + if (!sm6_type_is_numeric(value->type)) + { + WARN("Operand result type class %u is not numeric.\n", value->type->class); + vkd3d_shader_parser_error(&sm6->p, VKD3D_SHADER_ERROR_DXIL_INVALID_OPERAND, + "A numeric operand passed to a DXIL instruction is not numeric."); + return false; + } + return true; +} + static bool sm6_value_validate_is_bool(const struct sm6_value *value, struct sm6_parser *sm6) { const struct sm6_type *type = value->type; @@ -3989,6 +4001,57 @@ static void sm6_parser_emit_gep(struct sm6_parser *sm6, const struct dxil_record ins->handler_idx = VKD3DSIH_NOP; } +static void sm6_parser_emit_load(struct sm6_parser *sm6, const struct dxil_record *record, + struct vkd3d_shader_instruction *ins, struct sm6_value *dst) +{ + const struct sm6_type *elem_type = NULL, *pointee_type; + struct vkd3d_shader_src_param *src_param; + unsigned int alignment, i = 0; + const struct sm6_value *ptr; + uint64_t alignment_code; + + if (!(ptr = sm6_parser_get_value_by_ref(sm6, record, NULL, &i))) + return; + if (!sm6_value_validate_is_register(ptr, sm6) + || !sm6_value_validate_is_pointer(ptr, sm6) + || !dxil_record_validate_operand_count(record, i + 2, i + 3, sm6)) + return; + + if (record->operand_count > i + 2 && !(elem_type = sm6_parser_get_type(sm6, record->operands[i++]))) + return; + + if (!elem_type) + { + elem_type = ptr->type->u.pointer.type; + } + else if (elem_type != (pointee_type = ptr->type->u.pointer.type)) + { + WARN("Type mismatch.\n"); + vkd3d_shader_parser_warning(&sm6->p, VKD3D_SHADER_WARNING_DXIL_TYPE_MISMATCH, + "Type mismatch in pointer load arguments."); + } + + dst->type = elem_type; + + if (!sm6_value_validate_is_numeric(dst, sm6)) + return; + + alignment_code = record->operands[i++]; + if (!bitcode_parse_alignment(alignment_code, &alignment)) + WARN("Invalid alignment %"PRIu64".\n", alignment_code); + + if (record->operands[i]) + WARN("Ignoring volatile modifier.\n"); + + vsir_instruction_init(ins, &sm6->p.location, VKD3DSIH_MOV); + + src_param = instruction_src_params_alloc(ins, 1, sm6); + src_param_init_from_value(&src_param[0], ptr); + src_param->reg.alignment = alignment; + + instruction_dst_param_init_ssa_scalar(ins, sm6); +} + static void sm6_parser_emit_ret(struct sm6_parser *sm6, const struct dxil_record *record, struct sm6_block *code_block, struct vkd3d_shader_instruction *ins) { @@ -4194,6 +4257,9 @@ static enum vkd3d_result sm6_parser_function_init(struct sm6_parser *sm6, const case FUNC_CODE_INST_GEP: sm6_parser_emit_gep(sm6, record, ins, dst); break; + case FUNC_CODE_INST_LOAD: + sm6_parser_emit_load(sm6, record, ins, dst); + break; case FUNC_CODE_INST_RET: sm6_parser_emit_ret(sm6, record, code_block, ins); is_terminator = true; diff --git a/libs/vkd3d-shader/ir.c b/libs/vkd3d-shader/ir.c index ae1610483..f216c82f7 100644 --- a/libs/vkd3d-shader/ir.c +++ b/libs/vkd3d-shader/ir.c @@ -304,6 +304,7 @@ void vsir_register_init(struct vkd3d_shader_register *reg, enum vkd3d_shader_reg reg->idx[2].is_in_bounds = false; reg->idx_count = idx_count; reg->dimension = VSIR_DIMENSION_SCALAR; + reg->alignment = 0; } void vsir_instruction_init(struct vkd3d_shader_instruction *ins, const struct vkd3d_shader_location *location, diff --git a/libs/vkd3d-shader/vkd3d_shader_private.h b/libs/vkd3d-shader/vkd3d_shader_private.h index 68eef0b8d..d21ecc513 100644 --- a/libs/vkd3d-shader/vkd3d_shader_private.h +++ b/libs/vkd3d-shader/vkd3d_shader_private.h @@ -824,6 +824,8 @@ struct vkd3d_shader_register struct vkd3d_shader_register_index idx[3]; unsigned int idx_count; enum vsir_dimension dimension; + /* known address alignment for optimisation, or zero */ + unsigned int alignment; union { DWORD immconst_uint[VKD3D_VEC4_SIZE]; diff --git a/tests/hlsl/matrix-indexing.shader_test b/tests/hlsl/matrix-indexing.shader_test index b8e6dec68..bc4a46d03 100644 --- a/tests/hlsl/matrix-indexing.shader_test +++ b/tests/hlsl/matrix-indexing.shader_test @@ -120,8 +120,8 @@ float4 main() : sv_target [test] uniform 0 float 2 -todo(sm>=6) draw quad -probe all rgba (8, 9, 10, 11) +draw quad +todo(sm>=6) probe all rgba (8, 9, 10, 11) [pixel shader] @@ -136,5 +136,5 @@ float4 main() : sv_target [test] uniform 0 float 3 -todo(sm>=6) draw quad +draw quad todo(sm>=6) probe all rgba (12, 13, 14, 15) diff --git a/tests/hlsl/non-const-indexing.shader_test b/tests/hlsl/non-const-indexing.shader_test index 3a1e12acc..638107e79 100644 --- a/tests/hlsl/non-const-indexing.shader_test +++ b/tests/hlsl/non-const-indexing.shader_test @@ -36,17 +36,17 @@ float4 main() : SV_TARGET [test] uniform 0 float 0 -todo(sm>=6) draw quad -probe all rgba (11.0, 11.0, 11.0, 11.0) +draw quad +todo(sm>=6) probe all rgba (11.0, 11.0, 11.0, 11.0) uniform 0 float 1 -todo(sm>=6) draw quad -probe all rgba (12.0, 12.0, 12.0, 12.0) +draw quad +todo(sm>=6) probe all rgba (12.0, 12.0, 12.0, 12.0) uniform 0 float 2 -todo(sm>=6) draw quad -probe all rgba (13.0, 13.0, 13.0, 13.0) +draw quad +todo(sm>=6) probe all rgba (13.0, 13.0, 13.0, 13.0) uniform 0 float 3 -todo(sm>=6) draw quad -probe all rgba (14.0, 14.0, 14.0, 14.0) +draw quad +todo(sm>=6) probe all rgba (14.0, 14.0, 14.0, 14.0) [pixel shader] @@ -61,7 +61,7 @@ float4 main() : sv_target [test] uniform 0 float 2.3 -todo(sm>=6) draw quad +draw quad todo(sm>=6) probe all rgba (3, 3, 3, 3) @@ -77,17 +77,17 @@ float4 main() : SV_TARGET [test] uniform 0 float 0 -todo(sm>=6) draw quad -probe all rgba (21.0, 1.0, 24.0, 0.0) +draw quad +todo(sm>=6) probe all rgba (21.0, 1.0, 24.0, 0.0) uniform 0 float 1 -todo(sm>=6) draw quad -probe all rgba (22.0, 0.0, 23.0, 1.0) +draw quad +todo(sm>=6) probe all rgba (22.0, 0.0, 23.0, 1.0) uniform 0 float 2 -todo(sm>=6) draw quad -probe all rgba (23.0, 1.0, 22.0, 0.0) +draw quad +todo(sm>=6) probe all rgba (23.0, 1.0, 22.0, 0.0) uniform 0 float 3 -todo(sm>=6) draw quad -probe all rgba (24.0, 0.0, 21.0, 1.0) +draw quad +todo(sm>=6) probe all rgba (24.0, 0.0, 21.0, 1.0) [pixel shader] @@ -102,16 +102,16 @@ float4 main() : sv_target [test] uniform 0 float4 0 0 0 0 -todo(sm>=6) draw quad +draw quad todo(sm>=6) probe all rgba (1.0, 2.0, 3.0, 4.0) uniform 0 float4 1 0 0 0 -todo(sm>=6) draw quad +draw quad todo(sm>=6) probe all rgba (5.0, 6.0, 7.0, 8.0) uniform 0 float4 0 1 0 0 -todo(sm>=6) draw quad +draw quad todo(sm>=6) probe all rgba (5.0, 6.0, 7.0, 8.0) uniform 0 float4 1 1 0 0 -todo(sm>=6) draw quad +draw quad todo(sm>=6) probe all rgba (9.0, 10.0, 11.0, 12.0) @@ -130,8 +130,8 @@ float4 main() : sv_target [test] uniform 0 float4 0 0 2.4 0 -todo(sm>=6) draw quad -probe all rgba (1.0, 120.0, 90.0, 4.0) +draw quad +todo(sm>=6) probe all rgba (1.0, 120.0, 90.0, 4.0) % SM1 doesn't support relative addressing if it is used in a l-value. diff --git a/tests/hlsl/vector-indexing-uniform.shader_test b/tests/hlsl/vector-indexing-uniform.shader_test index 3501f3af7..d494d8626 100644 --- a/tests/hlsl/vector-indexing-uniform.shader_test +++ b/tests/hlsl/vector-indexing-uniform.shader_test @@ -12,5 +12,5 @@ float4 main() : SV_TARGET [test] uniform 0 float 2 -todo(sm>=6) draw quad -probe all rgba (0.5, 0.3, 0.8, 0.2) +draw quad +todo(sm>=6) probe all rgba (0.5, 0.3, 0.8, 0.2) -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/471
Changed FIXME to WARN for `sm6_type_get_pointer_to_type()` as it is elsewhere. Also needed to add a call to `sm6_value_validate_is_register()` for the LOAD instruction because a value of pointer type could be a function. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/471#note_53020
On Tue Nov 21 14:09:13 2023 +0000, Giovanni Mascellani wrote:
Fair point, you're right. Especially with the comments this is after all easier to follow for me. Sorry for the conflicting instructions. I think there's room for some subtlety here. Unsupported by the backend doesn't necessarily have to mean "ignored"; failing compilation when a non-zero alignment is encountered would certainly be a valid choice. In particular, that would avoid potentially miscompiling shaders.
-- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/471#note_53073
From patch 4/6: ```diff @@ -804,6 +804,7 @@ struct vkd3d_shader_indexable_temp unsigned int alignment; enum vkd3d_data_type data_type; unsigned int component_count; + const struct vkd3d_shader_immediate_constant_buffer *initialiser; };
struct vkd3d_shader_register_index ```
It looks like we don't output the initialiser in the d3d-asm output. Adding that seems helpful. And similar to e.g. "alignment", we probably want to fail compilation when the backend can't handle initialisers. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/471#note_53074
On Tue Nov 21 14:09:13 2023 +0000, Henri Verbeet wrote:
I think there's room for some subtlety here. Unsupported by the backend doesn't necessarily have to mean "ignored"; failing compilation when a non-zero alignment is encountered would certainly be a valid choice. In particular, that would avoid potentially miscompiling shaders. My understanding is the alignment is additional info to aid optimisation, and is not required for correct compilation. Should this patch add a WARN to the backend for a nonzero alignment?
-- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/471#note_53088
On Tue Nov 21 14:16:25 2023 +0000, Conor McCarthy wrote:
My understanding is the alignment is additional info to aid optimisation, and is not required for correct compilation. Should this patch add a WARN to the backend for a nonzero alignment? Oh right, that's probably fine then, although adding the WARN may still be helpful.
-- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/471#note_53118
participants (3)
-
Conor McCarthy -
Conor McCarthy (＠cmccarthy)
-
Henri Verbeet (＠hverbeet)