From 8a71f5e672963203626a84bf73d063286c3199ae Mon Sep 17 00:00:00 2001 From: Ian Romanick Date: Wed, 11 Jun 2025 07:33:37 -0700 Subject: [PATCH] brw: BFN does not support source modifiers Reviewed-by: Matt Turner Part-of: --- src/intel/compiler/brw/brw_disasm.c | 33 +++++++++++++++++++++--- src/intel/compiler/brw/brw_eu_inst.h | 1 + src/intel/compiler/brw/brw_eu_validate.c | 20 ++++++++++++-- src/intel/compiler/brw/brw_inst.cpp | 1 + src/intel/compiler/brw/brw_validate.cpp | 13 ++++++++++ 5 files changed, 62 insertions(+), 6 deletions(-) diff --git a/src/intel/compiler/brw/brw_disasm.c b/src/intel/compiler/brw/brw_disasm.c index 2a87259a9f0..8de18bfc628 100644 --- a/src/intel/compiler/brw/brw_disasm.c +++ b/src/intel/compiler/brw/brw_disasm.c @@ -2072,11 +2072,36 @@ brw_disassemble_inst(FILE *file, const struct brw_isa_info *isa, const unsigned rcount = brw_eu_inst_dpas_3src_rcount(devinfo, inst) + 1; format(file, "x%d", rcount); + } else if (opcode == BRW_OPCODE_BFN) { + unsigned cc; + + switch (brw_eu_inst_boolean_func_cond_modifier(devinfo, inst)) { + case 0: + cc = BRW_CONDITIONAL_NONE; + break; + case 1: + cc = BRW_CONDITIONAL_Z; + break; + case 2: + cc = BRW_CONDITIONAL_G; + break; + case 3: + cc = BRW_CONDITIONAL_L; + break; + } + + err |= control(file, "conditional modifier", conditional_modifier, + cc, NULL); + + /* If we're using the conditional modifier, print which flags reg is + * used for it. + */ + if (cc != BRW_CONDITIONAL_NONE) { + format(file, ".f%"PRIu64".%"PRIu64, + brw_eu_inst_flag_reg_nr(devinfo, inst), + brw_eu_inst_flag_subreg_nr(devinfo, inst)); + } } else if (!is_send(opcode) && - /* BFN has data in the place of the conditional modifier which - * is not a conditional modifer - */ - opcode != BRW_OPCODE_BFN && (devinfo->ver < 12 || brw_eu_inst_src0_reg_file(devinfo, inst) != IMM || brw_type_size_bytes(brw_eu_inst_src0_type(devinfo, inst)) < 8)) { diff --git a/src/intel/compiler/brw/brw_eu_inst.h b/src/intel/compiler/brw/brw_eu_inst.h index c61693c209d..868e6583a9c 100644 --- a/src/intel/compiler/brw/brw_eu_inst.h +++ b/src/intel/compiler/brw/brw_eu_inst.h @@ -521,6 +521,7 @@ F(3src_a1_src2_is_imm, /* 9+ */ -1, -1, /* 12+ */ 47, 47) F(3src_a1_src0_is_imm, /* 9+ */ -1, -1, /* 12+ */ 46, 46) FDC(boolean_func_ctrl, /* 9+ */ -1, -1, /* 12+ */ 95, 92, 87, 84, devinfo->verx10 >= 125) +F(boolean_func_cond_modifier,/* 9+ */ -1, -1, /* 12+ */ 45, 44) /* Source Modifier fields same in align16 */ FFC(3src_a1_dst_reg_file, /* 9+ */ 36, 36, /* 12+ */ 50, 50, devinfo->ver >= 10, .grf_or_acc = true) diff --git a/src/intel/compiler/brw/brw_eu_validate.c b/src/intel/compiler/brw/brw_eu_validate.c index 79f2549179b..25e2df96518 100644 --- a/src/intel/compiler/brw/brw_eu_validate.c +++ b/src/intel/compiler/brw/brw_eu_validate.c @@ -1970,6 +1970,9 @@ instruction_restrictions(const struct brw_isa_info *isa, ERROR_IF(inst->src[i].type != BRW_TYPE_UD && inst->src[i].type != BRW_TYPE_UW, "BFN source must be UD or UW type."); + + ERROR_IF(inst->src[i].abs || inst->src[i].negate, + "BFN does not support source modifiers."); } } @@ -2832,8 +2835,6 @@ brw_hw_decode_inst(const struct brw_isa_info *isa, inst->src[0].file = brw_eu_inst_3src_a1_src0_reg_file(devinfo, raw); inst->src[0].type = brw_eu_inst_3src_a1_src0_type(devinfo, raw); - inst->src[0].negate = brw_eu_inst_3src_src0_negate(devinfo, raw); - inst->src[0].abs = brw_eu_inst_3src_src0_abs(devinfo, raw); if (inst->src[0].file != IMM) { inst->src[0].nr = brw_eu_inst_3src_src0_reg_nr(devinfo, raw); inst->src[0].subnr = brw_eu_inst_3src_a1_src0_subreg_nr(devinfo, raw); @@ -2858,6 +2859,21 @@ brw_hw_decode_inst(const struct brw_isa_info *isa, inst->src[2].hstride = STRIDE(brw_eu_inst_3src_a1_src2_hstride(devinfo, raw)); inst->src[2].width = brw_implied_width_for_3src_a1(inst->src[2].vstride, inst->src[2].hstride); } + + switch (brw_eu_inst_boolean_func_cond_modifier(devinfo, raw)) { + case 0: + inst->cond_modifier = BRW_CONDITIONAL_NONE; + break; + case 1: + inst->cond_modifier = BRW_CONDITIONAL_Z; + break; + case 2: + inst->cond_modifier = BRW_CONDITIONAL_G; + break; + case 3: + inst->cond_modifier = BRW_CONDITIONAL_L; + break; + } break; } diff --git a/src/intel/compiler/brw/brw_inst.cpp b/src/intel/compiler/brw/brw_inst.cpp index 3789f28cca0..b26b537f3d9 100644 --- a/src/intel/compiler/brw/brw_inst.cpp +++ b/src/intel/compiler/brw/brw_inst.cpp @@ -336,6 +336,7 @@ brw_inst::can_do_source_mods(const struct intel_device_info *devinfo) const case BRW_OPCODE_BFE: case BRW_OPCODE_BFI1: case BRW_OPCODE_BFI2: + case BRW_OPCODE_BFN: case BRW_OPCODE_BFREV: case BRW_OPCODE_CBIT: case BRW_OPCODE_FBH: diff --git a/src/intel/compiler/brw/brw_validate.cpp b/src/intel/compiler/brw/brw_validate.cpp index ee53339a094..2d07f45728d 100644 --- a/src/intel/compiler/brw/brw_validate.cpp +++ b/src/intel/compiler/brw/brw_validate.cpp @@ -329,6 +329,19 @@ brw_validate(const brw_shader &s) break; } + case BRW_OPCODE_BFN: + VAL_ASSERT(!inst->src[0].abs && !inst->src[0].negate); + VAL_ASSERT(!inst->src[1].abs && !inst->src[1].negate); + VAL_ASSERT(!inst->src[2].abs && !inst->src[2].negate); + VAL_ASSERT_EQ(inst->src[3].file, IMM); + + /* BFN only supports a limited subset of conditions. */ + VAL_ASSERT(inst->conditional_mod == BRW_CONDITIONAL_NONE || + inst->conditional_mod == BRW_CONDITIONAL_Z || + inst->conditional_mod == BRW_CONDITIONAL_G || + inst->conditional_mod == BRW_CONDITIONAL_L); + break; + default: break; }