On Wed Mar 1 02:06:02 2023 +0000, Henri Verbeet wrote:
From patch 1/4:
+static void shader_instruction_eliminate_phase_instance_id(structvkd3d_shader_instruction *ins,
unsigned int instance_id)+{
- struct vkd3d_shader_register *reg;
- enum vkd3d_data_type data_type;
- unsigned int i;
- /* The dst always seems to be a TEMP, which are declared as
FLOAT. FLOAT always works in tests, but check
* for UINT for robustness. We assume the constant type shouldequal the dst type, which is not true for
* all instructions, but the compiler apparently never doesanything fancy with a fork/join instance id. */
- data_type = (ins->dst_count && ins->dst[0].reg.data_type ==
VKD3D_DATA_UINT) ? VKD3D_DATA_UINT : VKD3D_DATA_FLOAT;
Why do we care about the destination data type at all? Shouldn't we simply keep the existing reg->data_type of the source parameter we're replacing? More concretely, suppose vForkInstanceId is "1", what should the value of r0/1/2 be after the following instructions? mov r0, vForkInstanceId utof r1, vForkInstanceId ftou r2, vForkInstanceId I'd expect 0x00000001, 0x3f800000, and 0x00000000 respectively; is that incorrect? The critical—though perhaps subtle—thing to understand here is that vkd3d_shader_instruction registers don't have an inherent type, but are instead containers of raw bits; the "data_type" fields of the instruction's sources and destinations are a property of the instruction, not of the registers being referenced.
TBH I don't know why I was doing it this way; it's a legacy of the original which had SPIR-V type mismatches from this code. The data type is now left unchanged.