Zebediah Figura (@zfigura) commented about libs/vkd3d-shader/hlsl_codegen.c:
ret_swizzle |= value->component << HLSL_SWIZZLE_SHIFT(i); }
- /* When 'load' is not the same as 'instr', there is the possibility of 'load' itself to be - * 'new_instr'. In this case, the replacement is not necessary and we have to return false - * to avoid doing copy-prop forever. */ - if (new_instr == &load->node) + /* When 'load' is not the same as 'instr', there is the possibility of 'load' to be before in + * the program than 'new_instr', or even be equal to 'new_instr'. In this case, the replacement + * is not helpful, so we have to return false to avoid doing copy-prop forever. */ + if (load->node.index <= new_instr->index)
I fear this isn't going to be enough. E.g. does this handle loops correctly? Even if it is enough I feel like the logic is fragile enough that we should find some other way. Should we throw out this pass altogether? We have tests that depend on it, but I think those tests are probably very artificial. Alternatively: When we encounter a load, we record, in the copy_propagation_state, an array of copy_propagation_value's for that load. Then in copy_propagation_transform_swizzle(), instead of calling copy_propagation_get_value(), we use that value array. That way we are looking up the "live" values at the time of the load, rather than later after things may have been invalidated. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/482#note_52916