From a51d061c00b6b49853881783ffa02aca3ee105f4 Mon Sep 17 00:00:00 2001 From: Lionel Landwerlin Date: Mon, 2 Jun 2025 10:13:29 +0300 Subject: [PATCH] brw: don't generate invalid instructions 0e3e5146cf ("intel/brw: Use correct instruction for value change check when coalescing") enabled some new cases that exposed a pre-existing bug that would turn something like this : mul.sat(16) %789:F, %787:F, %788:F mov.g.f0.0(16) %790:F, %789:F (+f0.0) sel(16) %800:UD, %790:UD, 0u into this : mul.sat(16) %790:F, %787:F, %788:F mov.g.f0.0(16) null:F, null<8,8,1>:F (+f0.0) sel(16) %800:UD, %790:UD, 0u The mov[] array can contain the same instruction because it's repeated for each REG_SIZE writes and a SIMD16 instruction will write 2 REG_SIZE. Signed-off-by: Lionel Landwerlin Fixes: 0e3e5146cf ("intel/brw: Use correct instruction for value change check when coalescing") Cc: mesa-stable Reviewed-by: Matt Turner Reviewed-by: Ian Romanick Part-of: --- .../compiler/brw_opt_register_coalesce.cpp | 2 +- .../compiler/test_opt_register_coalesce.cpp | 50 +++++++++++++++++++ 2 files changed, 51 insertions(+), 1 deletion(-) diff --git a/src/intel/compiler/brw_opt_register_coalesce.cpp b/src/intel/compiler/brw_opt_register_coalesce.cpp index d00ea0b8c68..c4b9fff4b52 100644 --- a/src/intel/compiler/brw_opt_register_coalesce.cpp +++ b/src/intel/compiler/brw_opt_register_coalesce.cpp @@ -332,7 +332,7 @@ brw_opt_register_coalesce(brw_shader &s) progress = true; - for (int i = 0; i < src_size; i++) { + for (int i = 0; i < src_size; i += regs_written(mov[i])) { if (!mov[i]) continue; diff --git a/src/intel/compiler/test_opt_register_coalesce.cpp b/src/intel/compiler/test_opt_register_coalesce.cpp index aa47c082d26..57aa78a4be7 100644 --- a/src/intel/compiler/test_opt_register_coalesce.cpp +++ b/src/intel/compiler/test_opt_register_coalesce.cpp @@ -109,3 +109,53 @@ TEST_F(RegisterCoalesceTest, ChangingTemporaryCompoundRegisterNotChangesOriginal EXPECT_NO_PROGRESS(brw_opt_register_coalesce, bld); } + +TEST_F(RegisterCoalesceTest, MovWithFlagRegisterWrite) +{ + brw_builder bld = make_shader(MESA_SHADER_COMPUTE, 16); + brw_builder exp = make_shader(MESA_SHADER_COMPUTE, 16); + + /** + * mul.sat(16) %789:F, %787:F, %788:F + * mov.g.f0.0(16) %790:F, %789:F + * (+f0.0) sel(16) %800:UD, %790:UD, 0u + */ + + brw_reg src0 = vgrf(bld, exp, BRW_TYPE_F); + brw_reg src1 = vgrf(bld, exp, BRW_TYPE_F); + brw_reg vgrf0 = vgrf(bld, exp, BRW_TYPE_F); + brw_reg vgrf1 = vgrf(bld, exp, BRW_TYPE_F); + brw_reg vgrf2 = vgrf(bld, exp, BRW_TYPE_F); + + { + brw_inst *mul = bld.MUL(vgrf0, src0, src1); + mul->saturate = true; + + brw_inst *mov = bld.MOV(vgrf1, vgrf0); + mov->conditional_mod = BRW_CONDITIONAL_G; + + brw_inst *sel = bld.SEL(vgrf2, vgrf1, brw_imm_f(0.0)); + sel->predicate = BRW_PREDICATE_NORMAL; + } + + EXPECT_PROGRESS(brw_opt_register_coalesce, bld); + + /** + * mul.sat(16) %789:F, %787:F, %788:F + * mov.g.f0.0(16) null:F, %789:F + * (+f0.0) sel(16) %800:UD, %789:UD, 0u + */ + + { + brw_inst *mul = exp.MUL(vgrf0, src0, src1); + mul->saturate = true; + + brw_inst *mov = exp.MOV(brw_null_reg(), vgrf0); + mov->conditional_mod = BRW_CONDITIONAL_G; + + brw_inst *sel = exp.SEL(vgrf2, vgrf0, brw_imm_f(0.0)); + sel->predicate = BRW_PREDICATE_NORMAL; + } + + EXPECT_SHADERS_MATCH(bld, exp); +}