[PATCH vkd3d v2 3/6] vkd3d-shader: Introduce a descriptor array symbol type to struct vkd3d_symbol.

[Henri Verbeet]

On Mon, 19 Jul 2021 at 09:22, Conor McCarthy
<conor.mccarthy6 at bigpond.com> wrote:
> +static uint32_t vkd3d_dxbc_compiler_build_descriptor_variable(struct vkd3d_dxbc_compiler *compiler,
> +        SpvStorageClass storage_class, uint32_t type_id, const struct vkd3d_shader_register *reg,
> +        const struct vkd3d_shader_register_range *range, enum vkd3d_shader_resource_type resource_type,
> +        bool is_uav_counter, const struct vkd3d_symbol **array_symbol)
> +{
The "is_uav_counter" parameter is always false. (At least, until patch
6/6 in this series.)

> +    if (array_variable)
> +    {
> +        symbol.info.descriptor_array.contained_type_id = type_id;
> +        type_id = vkd3d_spirv_get_op_type_array(builder, type_id,
> +                vkd3d_dxbc_compiler_get_constant_uint(compiler, binding.count));
> +    }
> +
> +    ptr_type_id = vkd3d_spirv_get_op_type_pointer(builder, storage_class, type_id);
> +
> +    if (!array_variable)
> +    {
> +        uint32_t var_id = vkd3d_spirv_build_op_variable(builder, &builder->global_stream,
> +                ptr_type_id, storage_class, 0);
> +
> +        vkd3d_dxbc_compiler_emit_descriptor_binding(compiler, var_id, &binding);
> +        vkd3d_dxbc_compiler_emit_register_debug_name(builder, var_id, reg);
> +
> +        *array_symbol = NULL;
> +        return var_id;
> +    }
> +
"type_id" stores different things depending on whether
"array_variable" is true or not, and I don't think it's making the
code any clearer. It may be clearer to introduce a separate variable
for the array type id. I.e., like this:

    if (!array_variable)
    {
        ptr_type_id = vkd3d_spirv_build_op_variable(..., type_id);
        ...
        return var_id;
    }

    array_type_id = vkd3d_spirv_get_op_type_array(...);
    ptr_type_id = vkd3d_spirv_build_op_variable(..., array_type_id);

    ...
    return var_id;

> +    /* Declare one array variable per Vulkan binding, and use it for all array declarations
> +     * which map to it. In this case ptr_type_id must point to an array type. */
> +    symbol.type = VKD3D_SYMBOL_DESCRIPTOR_ARRAY;
> +    memset(&symbol.key, 0, sizeof(symbol.key));
> +    symbol.key.descriptor_array.ptr_type_id = ptr_type_id;
> +    symbol.key.descriptor_array.set = binding.set;
> +    symbol.key.descriptor_array.binding = binding.binding;
> +    if ((entry = rb_get(&compiler->symbol_table, &symbol)))
> +    {
> +        array_symbol_entry = RB_ENTRY_VALUE(entry, struct vkd3d_symbol, entry);
> +        *array_symbol = array_symbol_entry;
> +        return array_symbol_entry->id;
> +    }
> +

    *array_symbol = RB_ENTRY_VALUE(...);
    return (*array_symbol)->id;

right?