AlexIndustrial/mesa
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

brw: fix spilling for Xe2+

Browse Source 
The problem occurs with a series of instructions build the subgroup
invocation value :

mov(8)          g23<1>UW        0x76543210V
add(8)          g23.8<1>UW      g23<8,8,1>UW    0x0008UW
add(16)         g23.16<1>UW     g23<16,16,1>UW  0x0010UW

Our register spilling code operates on physical registers (64B on
Xe2+) and using the brw_inst::is_partial_write() helper only considers
32B registers. So the spiller doesn't see that the add(16) instruction
is doing a partial write and ends up discarding the previous value.

You can reproduce the issue by running a test like :

INTEL_DEBUG=spill_fs ./deqp-vk -n dEQP-VK.compute.pipeline.cooperative_matrix.khr_a.subgroupscope.constant.uint8_uint8.buffer.rowmajor.linear

Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
Fixes: aa494cbacf ("brw: align spilling offsets to physical register sizes")
Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/33642>
This commit is contained in:
Lionel Landwerlin
2025-02-20 15:23:04 +02:00
committed by Marge Bot
parent 602843a881
commit c60180ba63
3 changed files with 5 additions and 6 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/intel/compiler/brw_inst.cpp
+3 -4
View File
@@ -395,7 +395,7 @@ brw_inst::can_change_types() const
* it.
*/
bool
brw_inst::is_partial_write() const
brw_inst::is_partial_write(unsigned grf_size) const
{
if (this->predicate && !this->predicate_trivial &&
this->opcode != BRW_OPCODE_SEL)
@@ -404,10 +404,10 @@ brw_inst::is_partial_write() const
if (!this->dst.is_contiguous())
return true;
if (this->dst.offset % REG_SIZE != 0)
if (this->dst.offset % grf_size != 0)
return true;
return this->size_written % REG_SIZE != 0;
return this->size_written % grf_size != 0;
}
unsigned
@@ -1239,4 +1239,3 @@ is_coalescing_payload(const brw_shader &s, const brw_inst *inst)
inst->src[0].offset == 0 &&
s.alloc.sizes[inst->src[0].nr] * REG_SIZE == inst->size_written;
}
src/intel/compiler/brw_inst.h
+1 -1
View File
@@ -66,7 +66,7 @@ public:
bool is_send_from_grf() const;
bool is_payload(unsigned arg) const;
bool is_partial_write() const;
bool is_partial_write(unsigned grf_size = REG_SIZE) const;
unsigned components_read(unsigned i) const;
unsigned size_read(const struct intel_device_info *devinfo, int arg) const;
bool can_do_source_mods(const struct intel_device_info *devinfo) const;
src/intel/compiler/brw_reg_allocate.cpp
+1 -1
View File
@@ -1227,7 +1227,7 @@ brw_reg_alloc::spill_reg(unsigned spill_reg)
* write, there should be no need for the unspill since the
* instruction will be overwriting the whole destination in any case.
*/
if (inst->is_partial_write() ||
if (inst->is_partial_write(reg_unit(devinfo) * REG_SIZE) ||
(!inst->force_writemask_all && !per_channel))
emit_unspill(ubld, &fs->shader_stats, spill_src,
subset_spill_offset, regs_written(inst), ip);