brw/const: Don't allow type changes when accumulators are involved

Integer accumulators and float accumulators do not occupy the same bits,
so the types cannot be arbitrarily changed.

No shader-db or fossil-db changes on any Intel platform.

v2: Use is_accumulator() instead if brw_reg_is_arf(). Add an extra test
to show the desired behavior when an accumulator is not
involved. Suggested by Caio.

Fixes: 64c251bb3a ("intel/fs: Combine constants for SEL instructions too")
Reviewed-by: Caio Oliveira <caio.oliveira@intel.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/40638>
This commit is contained in:
Ian Romanick 2026-03-25 09:50:25 -07:00 committed by Marge Bot
parent 11c3173890
commit ffdc310bf1
2 changed files with 151 additions and 3 deletions

View file

@ -1149,14 +1149,18 @@ add_candidate_immediate(struct table *table, brw_inst *inst, unsigned ip,
}
/* It is safe to change the type of the operands of a select instruction
* that has no conditional modifier, no source modifiers, and no saturate
* modifer.
* that has no conditional modifier, no source modifiers, no saturate
* modifier, and no accumulators are used. The types of accumulator accesses
* cannot be arbitrarily changed.
*/
if (inst->opcode == BRW_OPCODE_SEL &&
inst->conditional_mod == BRW_CONDITIONAL_NONE &&
!inst->src[0].negate && !inst->src[0].abs &&
!inst->src[1].negate && !inst->src[1].abs &&
!inst->saturate) {
!inst->saturate &&
!inst->src[0].is_accumulator() &&
!inst->src[1].is_accumulator() &&
!inst->dst.is_accumulator()) {
v->type = either_type;
}
}

View file

@ -61,3 +61,147 @@ TEST_F(CombineConstantsTest, DoContainingDo)
EXPECT_SHADERS_MATCH(bld, exp);
}
TEST_F(CombineConstantsTest, sel_f_integer_negation)
{
brw_builder bld = make_shader(MESA_SHADER_COMPUTE);
brw_builder exp = make_shader(MESA_SHADER_COMPUTE);
brw_reg dst0 = vgrf(bld, exp, BRW_TYPE_F);
brw_reg dst1 = vgrf(bld, exp, BRW_TYPE_F);
brw_reg src0 = vgrf(bld, exp, BRW_TYPE_F);
brw_reg src1 = vgrf(bld, exp, BRW_TYPE_F);
brw_reg tmp1 = vgrf(bld, exp, BRW_TYPE_D);
/* src0/src1 are not relevant to the SEL instructions, so they are filled
* with garbage.
*/
bld.BFREV(retype(src0, BRW_TYPE_UD), brw_imm_ud(1));
bld.BFREV(retype(src1, BRW_TYPE_UD), brw_imm_ud(2));
bld.CMP(retype(brw_null_reg(), BRW_TYPE_F),
src0, brw_imm_f(0.0), BRW_CONDITIONAL_Z);
/* Both SEL instructions have F types. src0 of the first SEL and src1 of
* the second SEL are the integer negations of each other. Constant
* combining is expected to change the type on one of the SEL to D, store a
* single integer value in a register, and use integer negation in a SEL
* source to generate the other value.
*/
bld.SEL(dst0,
/* The type cast here enables using values that are integer negations
* of each other.
*/
retype(brw_imm_d(1), BRW_TYPE_F),
brw_imm_f(2.0))
->predicate = BRW_PREDICATE_NORMAL;
bld.CMP(retype(brw_null_reg(), BRW_TYPE_F),
src1, brw_imm_f(0.0), BRW_CONDITIONAL_Z);
bld.SEL(dst1,
/* The type cast here enables using values that are integer negations
* of each other.
*/
retype(brw_imm_d(-1), BRW_TYPE_F),
brw_imm_f(3.0))
->predicate = BRW_PREDICATE_NORMAL;
EXPECT_PROGRESS(brw_opt_combine_constants, bld);
exp.BFREV(retype(src0, BRW_TYPE_UD), brw_imm_ud(1));
exp.BFREV(retype(src1, BRW_TYPE_UD), brw_imm_ud(2));
exp.CMP(retype(brw_null_reg(), BRW_TYPE_F),
src0, brw_imm_f(0.0), BRW_CONDITIONAL_Z);
tmp1.stride = 0;
exp.uniform().MOV(tmp1, brw_imm_d(1));
exp.SEL(dst0,
retype(tmp1, BRW_TYPE_F),
brw_imm_f(2.0))
->predicate = BRW_PREDICATE_NORMAL;
exp.CMP(retype(brw_null_reg(), BRW_TYPE_F),
src1, brw_imm_f(0.0), BRW_CONDITIONAL_Z);
exp.SEL(retype(dst1, BRW_TYPE_D),
/* Note the negation here to convert 1 to -1. */
negate(tmp1),
retype(brw_imm_f(3.0), BRW_TYPE_D))
->predicate = BRW_PREDICATE_NORMAL;
EXPECT_SHADERS_MATCH(bld, exp);
}
TEST_F(CombineConstantsTest, sel_to_accumulator)
{
brw_builder bld = make_shader(MESA_SHADER_COMPUTE);
brw_builder exp = make_shader(MESA_SHADER_COMPUTE);
brw_reg src0 = vgrf(bld, exp, BRW_TYPE_F);
brw_reg src1 = vgrf(bld, exp, BRW_TYPE_F);
brw_reg tmp1 = vgrf(bld, exp, BRW_TYPE_D);
brw_reg acc0 = retype(brw_acc_reg(8 * reg_unit(devinfo)),
BRW_TYPE_F);
/* src0/src1 are not relevant to the SEL instructions, so they are filled
* with garbage.
*/
bld.BFREV(retype(src0, BRW_TYPE_UD), brw_imm_ud(1));
bld.BFREV(retype(src1, BRW_TYPE_UD), brw_imm_ud(2));
bld.CMP(retype(brw_null_reg(), BRW_TYPE_F),
src0, brw_imm_f(0.0), BRW_CONDITIONAL_Z);
/* Unlike in sel_f_integer_negation, the types of the SEL instructions
* cannot be changed to D because both SEL instructions write the
* accumulator. Integer accumulator and float accumulator do not map the
* bits the same way, so the types cannot be changed.
*/
bld.SEL(acc0,
/* The type cast here enables using values that are integer negations
* of each other.
*/
retype(brw_imm_d(1), BRW_TYPE_F),
brw_imm_f(2.0))
->predicate = BRW_PREDICATE_NORMAL;
bld.CMP(retype(brw_null_reg(), BRW_TYPE_F),
src1, brw_imm_f(0.0), BRW_CONDITIONAL_Z);
bld.SEL(acc0,
/* The type cast here enables using values that are integer negations
* of each other.
*/
retype(brw_imm_d(-1), BRW_TYPE_F),
brw_imm_f(3.0))
->predicate = BRW_PREDICATE_NORMAL;
EXPECT_PROGRESS(brw_opt_combine_constants, bld);
exp.BFREV(retype(src0, BRW_TYPE_UD), brw_imm_ud(1));
exp.BFREV(retype(src1, BRW_TYPE_UD), brw_imm_ud(2));
exp.CMP(retype(brw_null_reg(), BRW_TYPE_F),
src0, brw_imm_f(0.0), BRW_CONDITIONAL_Z);
tmp1.stride = 0;
exp.uniform().MOV(tmp1, brw_imm_d(1));
exp.SEL(acc0,
retype(tmp1, BRW_TYPE_F),
brw_imm_f(2.0))
->predicate = BRW_PREDICATE_NORMAL;
exp.CMP(retype(brw_null_reg(), BRW_TYPE_F),
src1, brw_imm_f(0.0), BRW_CONDITIONAL_Z);
tmp1.offset = 4;
exp.uniform().MOV(tmp1, brw_imm_d(-1));
exp.SEL(acc0,
retype(tmp1, BRW_TYPE_F),
brw_imm_f(3.0))
->predicate = BRW_PREDICATE_NORMAL;
EXPECT_SHADERS_MATCH(bld, exp);
}