On Tue Jul 11 01:41:58 2023 +0000, Henri Verbeet wrote:
+#define DATA_TYPE_WIDTH_MASK 0xffu +#define DATA_TYPE_CLASS_SHIFT 8u +#define DATA_TYPE_CLASS_MASK (0x3fu << DATA_TYPE_CLASS_SHIFT) +#define DATA_TYPE_NORM_SHIFT 14u +#define DATA_TYPE_NORM_MASK (1u << DATA_TYPE_NORM_SHIFT) +#define DATA_TYPE_IS_SIGNED_SHIFT 15u +#define DATA_TYPE_IS_SIGNED_MASK (1u << DATA_TYPE_IS_SIGNED_SHIFT) + +enum vkd3d_data_type_class +{ + VKD3D_DATA_CLASS_FLOAT, + VKD3D_DATA_CLASS_INT, + VKD3D_DATA_CLASS_RESOURCE, + VKD3D_DATA_CLASS_SAMPLER, + VKD3D_DATA_CLASS_UAV, + VKD3D_DATA_CLASS_OPAQUE, + VKD3D_DATA_CLASS_MIXED, + VKD3D_DATA_CLASS_CONTINUED, + VKD3D_DATA_CLASS_UNUSED, +}; + enum vkd3d_data_type { - VKD3D_DATA_FLOAT, - VKD3D_DATA_INT, - VKD3D_DATA_RESOURCE, - VKD3D_DATA_SAMPLER, - VKD3D_DATA_UAV, - VKD3D_DATA_UINT, - VKD3D_DATA_UNORM, - VKD3D_DATA_SNORM, - VKD3D_DATA_OPAQUE, - VKD3D_DATA_MIXED, - VKD3D_DATA_DOUBLE, - VKD3D_DATA_CONTINUED, - VKD3D_DATA_UNUSED, + VKD3D_DATA_FLOAT = (VKD3D_DATA_CLASS_FLOAT <<DATA_TYPE_CLASS_SHIFT) | 32,
- VKD3D_DATA_INT = (VKD3D_DATA_CLASS_INT <<
DATA_TYPE_CLASS_SHIFT) | 32 | DATA_TYPE_IS_SIGNED_MASK,
- VKD3D_DATA_RESOURCE = VKD3D_DATA_CLASS_RESOURCE << DATA_TYPE_CLASS_SHIFT,
- VKD3D_DATA_SAMPLER = VKD3D_DATA_CLASS_SAMPLER << DATA_TYPE_CLASS_SHIFT,
- VKD3D_DATA_UAV = VKD3D_DATA_CLASS_UAV << DATA_TYPE_CLASS_SHIFT,
- VKD3D_DATA_UINT = (VKD3D_DATA_CLASS_INT <<
DATA_TYPE_CLASS_SHIFT) | 32,
- VKD3D_DATA_UNORM = (VKD3D_DATA_CLASS_INT <<
DATA_TYPE_CLASS_SHIFT) | 32 | DATA_TYPE_NORM_MASK,
- VKD3D_DATA_SNORM = (VKD3D_DATA_CLASS_INT <<
DATA_TYPE_CLASS_SHIFT) | 32 | DATA_TYPE_NORM_MASK | DATA_TYPE_IS_SIGNED_MASK,
- VKD3D_DATA_OPAQUE = VKD3D_DATA_CLASS_OPAQUE << DATA_TYPE_CLASS_SHIFT,
- VKD3D_DATA_MIXED = VKD3D_DATA_CLASS_MIXED << DATA_TYPE_CLASS_SHIFT,
- VKD3D_DATA_DOUBLE = (VKD3D_DATA_CLASS_FLOAT <<
DATA_TYPE_CLASS_SHIFT) | 64,
- VKD3D_DATA_CONTINUED = VKD3D_DATA_CLASS_CONTINUED << DATA_TYPE_CLASS_SHIFT,
- VKD3D_DATA_UNUSED = VKD3D_DATA_CLASS_UNUSED << DATA_TYPE_CLASS_SHIFT,
};
I'll wait for @giomasce, and possibly @etang-cw to take a look at this before looking more closely myself, but from just a quick look, do we really need the shifts and masks here? It would seem more straightforward to just store the required information in a structure, possibly using bitfields if we're particularly concerned about the size of that structure. It's also not immediately clear to me from the commit messages what the benefit of using enum vkd3d_data_type instead of enum vkd3d_shader_component_type is. If it's just about supporting 16-bit and 64-bit components in struct signature_element, adding a "component_size" field or something similar would seem nicer.
My initial thought was to use bitfields. It would make it pretty ugly to use a single specific value to identify each type, and to initialise a type, but we can declare constant initialisers and types can be checked with helpers.
Signature elements are the only external source of `enum vkd3d_shader_component_type` values in the SPIR-V backend, while `enum vkd3d_data_type` is heavily used in registers. We already have numerous two-step conversions from data type to component type to SPIR-V type id, and in the SM 6 branch I added a `vkd3d_spirv_get_type_id_for_data_type()` function to avoid many more of these. Using an `enum vkd3d_data_type` in struct signature_element entirely removes the need for `enum vkd3d_shader_component_type` in the backend, which is the main reason for doing it.
The extra type sizes are making the code clunky in the SM 6 branch. For example, checking if a type is an integer is: `data_type == VKD3D_DATA_UINT16 || data_type == VKD3D_DATA_INT || data_type == VKD3D_DATA_UINT || data_type == VKD3D_DATA_UINT64`
These changes would make that check simple, and match the type + width pattern use both in DXIL and SPIR-V.