From 985ace332bc9d90fcbeca5e71aebfa7aea72afba Mon Sep 17 00:00:00 2001 From: Ian Romanick Date: Tue, 9 Dec 2025 11:53:00 -0800 Subject: [PATCH] brw/algebraic: Allow mixed types in saturate constant folding Prevents assertion failures in func.shader-ballot.basic.q0 and other tests starting with "nir/algebraic: Optimize some b2f of integer comparison". Vector immediates, bfloat, and 8-bit floats are still not supported. v2: Almost complete re-write based on suggestions from Ken. v3: Don't retype() on a brw_imm_f value. Fixes: f8e54d02f79 ("intel/compiler: Relax mixed type restriction for saturating immediates") Reviewed-by: Kenneth Graunke Part-of: --- src/intel/compiler/brw/brw_opt_algebraic.cpp | 41 +++++++----- src/intel/compiler/brw/brw_reg.cpp | 63 ------------------- src/intel/compiler/brw/brw_reg.h | 1 - src/intel/compiler/brw/test_opt_algebraic.cpp | 49 +++++++++++++++ 4 files changed, 76 insertions(+), 78 deletions(-) diff --git a/src/intel/compiler/brw/brw_opt_algebraic.cpp b/src/intel/compiler/brw/brw_opt_algebraic.cpp index ff1932e1def..5ce118afb01 100644 --- a/src/intel/compiler/brw/brw_opt_algebraic.cpp +++ b/src/intel/compiler/brw/brw_opt_algebraic.cpp @@ -411,22 +411,35 @@ brw_opt_algebraic(brw_shader &s) if (inst->src[0].file != IMM) break; - if (inst->saturate) { - /* Full mixed-type saturates don't happen. However, we can end up - * with things like: - * - * mov.sat(8) g21<1>DF -1F - * - * Other mixed-size-but-same-base-type cases may also be possible. - */ - if (inst->dst.type != inst->src[0].type && - inst->dst.type != BRW_TYPE_DF && - inst->src[0].type != BRW_TYPE_F) - UNREACHABLE("unimplemented: saturate mixed types"); - - if (brw_reg_saturate_immediate(&inst->src[0])) { + if (inst->saturate && brw_type_is_float(inst->dst.type)) { + switch (inst->src[0].type) { + case BRW_TYPE_UW: + case BRW_TYPE_UD: + case BRW_TYPE_UQ: + inst->src[0] = brw_imm_f(src_as_uint(inst->src[0]) != 0); inst->saturate = false; progress = true; + break; + + case BRW_TYPE_W: + case BRW_TYPE_D: + case BRW_TYPE_Q: + inst->src[0] = brw_imm_f((int64_t)src_as_uint(inst->src[0]) > 0); + inst->saturate = false; + progress = true; + break; + + case BRW_TYPE_HF: + case BRW_TYPE_F: + case BRW_TYPE_DF: + inst->src[0] = brw_imm_f(SATURATE(src_as_float(inst->src[0]))); + inst->saturate = false; + progress = true; + break; + + default: + /* VF, BF, BF8, HF8 and others are not handled. */ + break; } } break; diff --git a/src/intel/compiler/brw/brw_reg.cpp b/src/intel/compiler/brw/brw_reg.cpp index 4cc9bceaaef..8e1f468506f 100644 --- a/src/intel/compiler/brw/brw_reg.cpp +++ b/src/intel/compiler/brw/brw_reg.cpp @@ -8,69 +8,6 @@ #include "brw_reg.h" #include "util/macros.h" -bool -brw_reg_saturate_immediate(brw_reg *reg) -{ - union { - unsigned ud; - int d; - float f; - double df; - } imm, sat_imm = { 0 }; - - const unsigned size = brw_type_size_bytes(reg->type); - - /* We want to either do a 32-bit or 64-bit data copy, the type is otherwise - * irrelevant, so just check the size of the type and copy from/to an - * appropriately sized field. - */ - if (size < 8) - imm.ud = reg->ud; - else - imm.df = reg->df; - - switch (reg->type) { - case BRW_TYPE_UD: - case BRW_TYPE_D: - case BRW_TYPE_UW: - case BRW_TYPE_W: - case BRW_TYPE_UQ: - case BRW_TYPE_Q: - /* Nothing to do. */ - return false; - case BRW_TYPE_F: - sat_imm.f = SATURATE(imm.f); - break; - case BRW_TYPE_DF: - sat_imm.df = SATURATE(imm.df); - break; - case BRW_TYPE_UB: - case BRW_TYPE_B: - UNREACHABLE("no UB/B immediates"); - case BRW_TYPE_V: - case BRW_TYPE_UV: - case BRW_TYPE_VF: - UNREACHABLE("unimplemented: saturate vector immediate"); - case BRW_TYPE_HF: - UNREACHABLE("unimplemented: saturate HF immediate"); - default: - UNREACHABLE("invalid type"); - } - - if (size < 8) { - if (imm.ud != sat_imm.ud) { - reg->ud = sat_imm.ud; - return true; - } - } else { - if (imm.df != sat_imm.df) { - reg->df = sat_imm.df; - return true; - } - } - return false; -} - bool brw_reg_negate_immediate(brw_reg *reg) { diff --git a/src/intel/compiler/brw/brw_reg.h b/src/intel/compiler/brw/brw_reg.h index 27cca76fdaf..6f7cb30bc6b 100644 --- a/src/intel/compiler/brw/brw_reg.h +++ b/src/intel/compiler/brw/brw_reg.h @@ -1322,7 +1322,6 @@ element_sz(struct brw_reg reg) int brw_float_to_vf(float f); float brw_vf_to_float(unsigned char vf); -bool brw_reg_saturate_immediate(brw_reg *reg); bool brw_reg_negate_immediate(brw_reg *reg); bool brw_reg_abs_immediate(brw_reg *reg); diff --git a/src/intel/compiler/brw/test_opt_algebraic.cpp b/src/intel/compiler/brw/test_opt_algebraic.cpp index 68478600425..ea8b18b387b 100644 --- a/src/intel/compiler/brw/test_opt_algebraic.cpp +++ b/src/intel/compiler/brw/test_opt_algebraic.cpp @@ -96,4 +96,53 @@ TEST_F(algebraic_test, mad_with_all_immediates) EXPECT_SHADERS_MATCH(bld, exp); } +TEST_F(algebraic_test, mov_sat_neg_constant_type_d_conversion) +{ + brw_builder bld = make_shader(MESA_SHADER_FRAGMENT, 16); + brw_builder exp = make_shader(MESA_SHADER_FRAGMENT, 16); + brw_reg dst0 = vgrf(bld, exp, BRW_TYPE_F); + + bld.MOV(dst0, brw_imm_d(-5)) + ->saturate = true; + + EXPECT_PROGRESS(brw_opt_algebraic, bld); + + exp.MOV(dst0, brw_imm_f(0.0)); + + EXPECT_SHADERS_MATCH(bld, exp); +} + +TEST_F(algebraic_test, mov_sat_neg_constant_type_ud_conversion) +{ + brw_builder bld = make_shader(MESA_SHADER_FRAGMENT, 16); + brw_builder exp = make_shader(MESA_SHADER_FRAGMENT, 16); + + brw_reg dst0 = vgrf(bld, exp, BRW_TYPE_F); + + bld.MOV(dst0, brw_imm_ud(-5)) + ->saturate = true; + + EXPECT_PROGRESS(brw_opt_algebraic, bld); + + exp.MOV(dst0, brw_imm_f(1.0)); + + EXPECT_SHADERS_MATCH(bld, exp); +} + +TEST_F(algebraic_test, mov_sat_pos_constant_type_conversion) +{ + brw_builder bld = make_shader(MESA_SHADER_FRAGMENT, 16); + brw_builder exp = make_shader(MESA_SHADER_FRAGMENT, 16); + + brw_reg dst0 = vgrf(bld, exp, BRW_TYPE_F); + + bld.MOV(dst0, brw_imm_ud(37)) + ->saturate = true; + + EXPECT_PROGRESS(brw_opt_algebraic, bld); + + exp.MOV(dst0, brw_imm_f(1.0)); + + EXPECT_SHADERS_MATCH(bld, exp); +}