From 80a5d158ae7d675e2cc62c78970a84abc1c85cfe Mon Sep 17 00:00:00 2001 From: Ian Romanick Date: Tue, 5 Nov 2024 14:37:55 -0800 Subject: [PATCH] brw/copy: Don't copy propagate through smaller entry dest size Copy propagation would incorrectly occur in this code mov(16) v4+2.0:UW, u0<0>:UW NoMask ... mov(8) v6+2.0:UD, v4+2.0:UD NoMask group0 to create mov(16) v4+2.0:UW, u0<0>:UW NoMask ... mov(8) v6+2.0:UD, u0<0>:UD NoMask group0 This has different behavior. I think I just made a mistake when I changed this condition in e3f502e0074. It seems like this condition could be relaxed to cover cases like (note the change of destination stride) mov(16) v4+2.0<2>:UW, u0<0>:UW NoMask ... mov(8) v6+2.0:UD, v4+2.0:UD NoMask group0 I'm not sure it's worth it. No shader-db or fossil-db changes on any Intel platform. Even the code for the test case mentioned in the original commit did not change. Reviewed-by: Kenneth Graunke Fixes: e3f502e0074 ("intel/fs: Allow copy propagation between MOVs of mixed sizes") Closes: #12116 Part-of: --- src/intel/compiler/brw_fs_copy_propagation.cpp | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/intel/compiler/brw_fs_copy_propagation.cpp b/src/intel/compiler/brw_fs_copy_propagation.cpp index 92c00760986..d9dcc72adba 100644 --- a/src/intel/compiler/brw_fs_copy_propagation.cpp +++ b/src/intel/compiler/brw_fs_copy_propagation.cpp @@ -825,9 +825,8 @@ try_copy_propagate(const brw_compiler *compiler, fs_inst *inst, * destination of the copy, and simply replacing the sources would give a * program with different semantics. */ - if ((brw_type_size_bits(entry->dst.type) < brw_type_size_bits(inst->src[arg].type) || - entry->is_partial_write) && - inst->opcode != BRW_OPCODE_MOV) { + if (brw_type_size_bits(entry->dst.type) < brw_type_size_bits(inst->src[arg].type) || + (entry->is_partial_write && inst->opcode != BRW_OPCODE_MOV)) { return false; } @@ -1506,8 +1505,7 @@ try_copy_propagate_def(const brw_compiler *compiler, * destination of the copy, and simply replacing the sources would give a * program with different semantics. */ - if (inst->opcode != BRW_OPCODE_MOV && - brw_type_size_bits(def->dst.type) < + if (brw_type_size_bits(def->dst.type) < brw_type_size_bits(inst->src[arg].type)) return false;