intel/brw: Fix handling of accumulator register numbers

Folks, there's more than one accumulator. In general, when the
register file is ARF, the upper 4 bits of the register number specify
which ARF, and the lower 4 bits specify which one of that ARF. This
can be further partitioned by the subregister number.

This is already mostly handled correctly for flags register, but lots
of places wanted to check the register number for equality with
BRW_ARF_ACCUMULATOR. If acc1 is ever specified, that won't work.

Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
Reviewed-by: Caio Oliveira <caio.oliveira@intel.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/28281>
This commit is contained in:
Ian Romanick
2023-08-09 14:03:57 -07:00
parent d8f53f698c
commit 66dc6e07f5
4 changed files with 26 additions and 23 deletions

View File

@@ -276,7 +276,7 @@ brw_set_src1(struct brw_codegen *p, brw_inst *inst, struct brw_reg reg)
* operands only." * operands only."
*/ */
assert(reg.file != BRW_ARCHITECTURE_REGISTER_FILE || assert(reg.file != BRW_ARCHITECTURE_REGISTER_FILE ||
reg.nr != BRW_ARF_ACCUMULATOR); (reg.nr & 0xF0) != BRW_ARF_ACCUMULATOR);
brw_inst_set_src1_file_type(devinfo, inst, reg.file, reg.type); brw_inst_set_src1_file_type(devinfo, inst, reg.file, reg.type);
brw_inst_set_src1_abs(devinfo, inst, reg.abs); brw_inst_set_src1_abs(devinfo, inst, reg.abs);
@@ -600,24 +600,19 @@ brw_alu3(struct brw_codegen *p, unsigned opcode, struct brw_reg dest,
if (brw_inst_access_mode(devinfo, inst) == BRW_ALIGN_1) { if (brw_inst_access_mode(devinfo, inst) == BRW_ALIGN_1) {
assert(dest.file == BRW_GENERAL_REGISTER_FILE || assert(dest.file == BRW_GENERAL_REGISTER_FILE ||
(dest.file == BRW_ARCHITECTURE_REGISTER_FILE && (dest.file == BRW_ARCHITECTURE_REGISTER_FILE &&
dest.nr == BRW_ARF_ACCUMULATOR)); (dest.nr & 0xF0) == BRW_ARF_ACCUMULATOR));
if (devinfo->ver >= 12) { STATIC_ASSERT((BRW_ARCHITECTURE_REGISTER_FILE ^ 1) == BRW_ALIGN1_3SRC_ACCUMULATOR);
brw_inst_set_3src_a1_dst_reg_file(devinfo, inst, dest.file); STATIC_ASSERT((BRW_GENERAL_REGISTER_FILE ^ 1) == BRW_ALIGN1_3SRC_GENERAL_REGISTER_FILE);
brw_inst_set_3src_dst_reg_nr(devinfo, inst, phys_nr(devinfo, dest));
} else { /* Gfx10 and Gfx11 bit encoding for the register file is the inversion of
if (dest.file == BRW_ARCHITECTURE_REGISTER_FILE) { * the actual register file (see the STATIC_ASSERTs above).
brw_inst_set_3src_a1_dst_reg_file(devinfo, inst, */
BRW_ALIGN1_3SRC_ACCUMULATOR); unsigned dst_reg_file = devinfo->ver >= 12 ? dest.file : dest.file ^ 1;
brw_inst_set_3src_dst_reg_nr(devinfo, inst, BRW_ARF_ACCUMULATOR);
} else { brw_inst_set_3src_a1_dst_reg_file(devinfo, inst, dst_reg_file);
brw_inst_set_3src_a1_dst_reg_file(devinfo, inst, brw_inst_set_3src_dst_reg_nr(devinfo, inst, phys_nr(devinfo, dest));
BRW_ALIGN1_3SRC_GENERAL_REGISTER_FILE);
brw_inst_set_3src_dst_reg_nr(devinfo, inst, dest.nr);
}
}
brw_inst_set_3src_a1_dst_subreg_nr(devinfo, inst, phys_subnr(devinfo, dest) / 8); brw_inst_set_3src_a1_dst_subreg_nr(devinfo, inst, phys_subnr(devinfo, dest) / 8);
brw_inst_set_3src_a1_dst_hstride(devinfo, inst, BRW_ALIGN1_3SRC_DST_HORIZONTAL_STRIDE_1); brw_inst_set_3src_a1_dst_hstride(devinfo, inst, BRW_ALIGN1_3SRC_DST_HORIZONTAL_STRIDE_1);
if (brw_reg_type_is_floating_point(dest.type)) { if (brw_reg_type_is_floating_point(dest.type)) {

View File

@@ -1732,7 +1732,7 @@ special_requirements_for_handling_double_precision_data_types(
ERROR_IF((address_mode == BRW_ADDRESS_DIRECT && file == BRW_ARCHITECTURE_REGISTER_FILE && ERROR_IF((address_mode == BRW_ADDRESS_DIRECT && file == BRW_ARCHITECTURE_REGISTER_FILE &&
reg != BRW_ARF_NULL && !(reg >= BRW_ARF_ACCUMULATOR && reg < BRW_ARF_FLAG)) || reg != BRW_ARF_NULL && !(reg >= BRW_ARF_ACCUMULATOR && reg < BRW_ARF_FLAG)) ||
(dst_file == BRW_ARCHITECTURE_REGISTER_FILE && (dst_file == BRW_ARCHITECTURE_REGISTER_FILE &&
dst_reg != BRW_ARF_NULL && dst_reg != BRW_ARF_ACCUMULATOR), dst_reg != BRW_ARF_NULL && (dst_reg & 0xF0) != BRW_ARF_ACCUMULATOR),
"Explicit ARF registers except null and accumulator must not " "Explicit ARF registers except null and accumulator must not "
"be used."); "be used.");
} }

View File

@@ -2538,7 +2538,7 @@ fs_visitor::dump_instruction_to_file(const fs_inst *inst, FILE *file) const
fprintf(file, "***attr%d***", inst->dst.nr); fprintf(file, "***attr%d***", inst->dst.nr);
break; break;
case ARF: case ARF:
switch (inst->dst.nr) { switch (inst->dst.nr & 0xF0) {
case BRW_ARF_NULL: case BRW_ARF_NULL:
fprintf(file, "null"); fprintf(file, "null");
break; break;
@@ -2546,7 +2546,11 @@ fs_visitor::dump_instruction_to_file(const fs_inst *inst, FILE *file) const
fprintf(file, "a0.%d", inst->dst.subnr); fprintf(file, "a0.%d", inst->dst.subnr);
break; break;
case BRW_ARF_ACCUMULATOR: case BRW_ARF_ACCUMULATOR:
fprintf(file, "acc%d", inst->dst.subnr); if (inst->dst.subnr == 0)
fprintf(file, "acc%d", inst->dst.nr & 0x0F);
else
fprintf(file, "acc%d.%d", inst->dst.nr & 0x0F, inst->dst.subnr);
break; break;
case BRW_ARF_FLAG: case BRW_ARF_FLAG:
fprintf(file, "f%d.%d", inst->dst.nr & 0xf, inst->dst.subnr); fprintf(file, "f%d.%d", inst->dst.nr & 0xf, inst->dst.subnr);
@@ -2636,7 +2640,7 @@ fs_visitor::dump_instruction_to_file(const fs_inst *inst, FILE *file) const
} }
break; break;
case ARF: case ARF:
switch (inst->src[i].nr) { switch (inst->src[i].nr & 0xF0) {
case BRW_ARF_NULL: case BRW_ARF_NULL:
fprintf(file, "null"); fprintf(file, "null");
break; break;
@@ -2644,7 +2648,11 @@ fs_visitor::dump_instruction_to_file(const fs_inst *inst, FILE *file) const
fprintf(file, "a0.%d", inst->src[i].subnr); fprintf(file, "a0.%d", inst->src[i].subnr);
break; break;
case BRW_ARF_ACCUMULATOR: case BRW_ARF_ACCUMULATOR:
fprintf(file, "acc%d", inst->src[i].subnr); if (inst->src[i].subnr == 0)
fprintf(file, "acc%d", inst->src[i].nr & 0x0F);
else
fprintf(file, "acc%d.%d", inst->src[i].nr & 0x0F, inst->src[i].subnr);
break; break;
case BRW_ARF_FLAG: case BRW_ARF_FLAG:
fprintf(file, "f%d.%d", inst->src[i].nr & 0xf, inst->src[i].subnr); fprintf(file, "f%d.%d", inst->src[i].nr & 0xf, inst->src[i].subnr);

View File

@@ -282,7 +282,7 @@ fs_reg::is_null() const
bool bool
fs_reg::is_accumulator() const fs_reg::is_accumulator() const
{ {
return file == ARF && nr == BRW_ARF_ACCUMULATOR; return file == ARF && (nr & 0xF0) == BRW_ARF_ACCUMULATOR;
} }
bool bool