Re: [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
 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?