Re: [PATCH v4 0/9] MR319: vkd3d-shader: Introduce vkd3d_shader_register.dimension, part 1.

[Henri Verbeet (＠hverbeet)]

This seems largely fine; couple of comments:
> - Ex 5/5 was moved and spliced into in 2/9 and 3/9. I can split 3/9 further but I didn't want to end up with too many patches.

I don't think ending up with too many patches is ever a concern.
> Subject: [PATCH 2/9] vkd3d-shader/ir: Use vsir_register_init() in
>  shader_register_normalise_flat_constants().

That changes behaviour though; shader_register_normalise_flat_constants() is modifying existing registers, not creating new ones. Arguably it's benign, but I'd still say it's wrong.
> -    memset(&r, 0, sizeof(r));
> -    r.type = VKD3DSPR_OUTPOINTID;
> -    r.idx[0].offset = ~0u;
> -    r.idx[1].offset = ~0u;
> -    r.idx_count = 0;
> +    vsir_register_init(&r, VKD3DSPR_OUTPOINTID, 0, 0);

Whether right or wrong, 0 above is VKD3D_DATA_FLOAT. That happens in a couple of other places as well. (I'd argue for "wrong, but largely irrelevant" for most of these; we could fix them up in follow-up patches if we care.)
> +static enum vkd3d_shader_dimension get_vkd3d_shader_dimension_from_sm4_dimension(enum vkd3d_sm4_dimension dim)
> +{
> +    switch (dim)
> +    {
> +        case VKD3D_SM4_DIMENSION_NONE:
> +            return VKD3D_SHADER_DIMENSION_NONE;
> +        case VKD3D_SM4_DIMENSION_SCALAR:
> +            return VKD3D_SHADER_DIMENSION_SCALAR;
> +        case VKD3D_SM4_DIMENSION_VEC4:
> +            return VKD3D_SHADER_DIMENSION_VEC4;
> +        default:
> +            FIXME("Unknown SM4 dimension %#x.\n", dim);
> +            return VKD3D_SHADER_DIMENSION_NONE;
> +    }
> +}

I'd argue for "vsir_dimension_from_sm4()".