intel/brw/xe2+: Implement Wa 22016140776

HF sources to math instructions cannot be scalar. This is very similar
to an old Gfx6 restriction on POW, so let's fix it in a similar way.

As an extra bit of saftey, lower any occurances that might slip through
in brw_fs_lower_regioning.

The primary change is to prevent copy propagation from violating the
restriction. With that change, nothing should be able to generate these
invalid source strides. The modification to fs_visitor::validate should
detect potential problems sooner rather than later.

Previous attempts to implement this Wa when emitting the math
instruction (in brw_eu_emit.c gfx6_math) didn't work for several
reasons. The lowering happens after the SWSB pass, so the scoreboarding
was incorrect (thanks to Curro for finding that). In addition, the
lowering happens after register allocation, so it's impossible to
allocate a non-scalar register to expand the scalar value.

Fixes 113 tests in the dEQP-VK.spirv_assembly.* group on LNL.

v2: Add changes to brw_fs_lower_regioning. Suggested by Curro.

Reviewed-by: Francisco Jerez <currojerez@riseup.net>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/28480>
This commit is contained in:
Ian Romanick 2024-03-28 09:54:20 -07:00
parent 50c7d25a9e
commit 0e817ba548
4 changed files with 73 additions and 8 deletions

View file

@ -769,6 +769,28 @@ general_restrictions_based_on_operand_types(const struct brw_isa_info *isa,
if (desc->ndst == 0)
return error_msg;
if (brw_inst_opcode(isa, inst) == BRW_OPCODE_MATH &&
intel_needs_workaround(devinfo, 22016140776)) {
/* Wa_22016140776:
*
* Scalar broadcast on HF math (packed or unpacked) must not be
* used. Compiler must use a mov instruction to expand the scalar
* value to a vector before using in a HF (packed or unpacked)
* math operation.
*/
ERROR_IF(brw_inst_src0_type(devinfo, inst) == BRW_REGISTER_TYPE_HF &&
src0_has_scalar_region(devinfo, inst),
"Scalar broadcast on HF math (packed or unpacked) must not "
"be used.");
if (num_sources > 1) {
ERROR_IF(brw_inst_src1_type(devinfo, inst) == BRW_REGISTER_TYPE_HF &&
src1_has_scalar_region(devinfo, inst),
"Scalar broadcast on HF math (packed or unpacked) must not "
"be used.");
}
}
/* The PRMs say:
*
* Where n is the largest element size in bytes for any source or

View file

@ -613,15 +613,29 @@ can_take_stride(fs_inst *inst, brw_reg_type dst_type,
return stride == 1 || stride == 0;
}
/* From the Broadwell PRM, Volume 2a "Command Reference - Instructions",
* page 391 ("Extended Math Function"):
*
* The following restrictions apply for align1 mode: Scalar source is
* supported. Source and destination horizontal stride must be the
* same.
*/
if (inst->is_math())
if (inst->is_math()) {
/* Wa_22016140776:
*
* Scalar broadcast on HF math (packed or unpacked) must not be used.
* Compiler must use a mov instruction to expand the scalar value to
* a vector before using in a HF (packed or unpacked) math operation.
*
* Prevent copy propagating a scalar value into a math instruction.
*/
if (intel_needs_workaround(devinfo, 22016140776) &&
stride == 0 && inst->src[arg].type == BRW_REGISTER_TYPE_HF) {
return false;
}
/* From the Broadwell PRM, Volume 2a "Command Reference - Instructions",
* page 391 ("Extended Math Function"):
*
* The following restrictions apply for align1 mode: Scalar source
* is supported. Source and destination horizontal stride must be
* the same.
*/
return stride == inst->dst.stride || stride == 0;
}
return true;
}

View file

@ -238,6 +238,17 @@ namespace {
has_invalid_src_region(const intel_device_info *devinfo, const fs_inst *inst,
unsigned i)
{
/* Wa_22016140776:
*
* Scalar broadcast on HF math (packed or unpacked) must not be used.
* Compiler must use a mov instruction to expand the scalar value to
* a vector before using in a HF (packed or unpacked) math operation.
*/
if (intel_needs_workaround(devinfo, 22016140776) &&
is_uniform(inst->src[i]) && inst->src[i].type == BRW_REGISTER_TYPE_HF) {
return true;
}
if (is_send(inst) || inst->is_math() || inst->is_control_source(i) ||
inst->opcode == BRW_OPCODE_DPAS) {
return false;

View file

@ -194,6 +194,24 @@ fs_visitor::validate()
phys_subnr(devinfo, inst->dst.as_brw_reg()) == 0) {
fsv_assert_eq(inst->dst.hstride, 1);
}
if (inst->is_math() && intel_needs_workaround(devinfo, 22016140776)) {
/* Wa_22016140776:
*
* Scalar broadcast on HF math (packed or unpacked) must not be
* used. Compiler must use a mov instruction to expand the scalar
* value to a vector before using in a HF (packed or unpacked)
* math operation.
*
* Since copy propagation knows about this restriction, nothing
* should be able to generate these invalid source strides. Detect
* potential problems sooner rather than later.
*/
for (unsigned i = 0; i < inst->sources; i++) {
fsv_assert(!is_uniform(inst->src[i]) ||
inst->src[i].type != BRW_REGISTER_TYPE_HF);
}
}
}
}
#endif