From 0ad835a929a243249e90a2c4148f5c2ec64be0eb Mon Sep 17 00:00:00 2001 From: Francisco Jerez Date: Sat, 9 Nov 2024 15:15:33 -0800 Subject: [PATCH] intel/fs/xe2: Fix up subdword integer region restriction with strided byte src and packed byte dst. This fixes a corner case of the LNL sub-dword integer restrictions that wasn't being detected by has_subdword_integer_region_restriction(), specifically: > if(Src.Type==Byte && Dst.Type==Byte && Dst.Stride==1 && W!=2) { > // ... > if(Src.Stride == 2) && (Src.UniformStride) && (Dst.SubReg%32 == Src.SubReg/2 ) { Allowed } > // ... > } All the other restrictions that require agreement between the SubReg number of source and destination only affect sources with a stride greater than a dword, which is why has_subdword_integer_region_restriction() was returning false except when "byte_stride(srcs[i]) >= 4" evaluated to true, but as implied by the pseudocode above, in the particular case of a packed byte destination, the restriction applies for source strides as narrow as 2B. The form of the equation that relates the subreg numbers is consistent with the existing calculations in brw_fs_lower_regioning (see required_src_byte_offset()), we just need to enable lowering for this corner case, and change lower_dst_region() to call lower_instruction() recursively, since some of the cases where we break this restriction are copy instructions introduced by brw_fs_lower_regioning() itself trying to lower other instructions with byte destinations. This fixes some Vulkan CTS test-cases that were hitting these restrictions with byte data types. Fixes: 217d41236076280 ("intel/fs/gfx20+: Implement sub-dword integer regioning restrictions.") Reviewed-by: Caio Oliveira Part-of: --- src/intel/compiler/brw_fs_lower_regioning.cpp | 23 +++++++++++-------- src/intel/compiler/brw_ir_fs.h | 8 +++++-- 2 files changed, 20 insertions(+), 11 deletions(-) diff --git a/src/intel/compiler/brw_fs_lower_regioning.cpp b/src/intel/compiler/brw_fs_lower_regioning.cpp index 89f3d20171a..e0a10e17cde 100644 --- a/src/intel/compiler/brw_fs_lower_regioning.cpp +++ b/src/intel/compiler/brw_fs_lower_regioning.cpp @@ -58,9 +58,8 @@ namespace { return MAX2(brw_type_size_bytes(inst->dst.type), byte_stride(inst->dst)); - } else if (has_subdword_integer_region_restriction(devinfo, inst) && - brw_type_size_bytes(inst->src[i].type) < 4 && - byte_stride(inst->src[i]) >= 4) { + } else if (has_subdword_integer_region_restriction(devinfo, inst, + &inst->src[i], 1)) { /* Use a stride of 32bits if possible, since that will guarantee that * the copy emitted to lower this region won't be affected by the * sub-dword integer region restrictions. This may not be possible @@ -85,9 +84,8 @@ namespace { if (has_dst_aligned_region_restriction(devinfo, inst)) { return reg_offset(inst->dst) % (reg_unit(devinfo) * REG_SIZE); - } else if (has_subdword_integer_region_restriction(devinfo, inst) && - brw_type_size_bytes(inst->src[i].type) < 4 && - byte_stride(inst->src[i]) >= 4) { + } else if (has_subdword_integer_region_restriction(devinfo, inst, + &inst->src[i], 1)) { const unsigned dst_byte_stride = MAX2(byte_stride(inst->dst), brw_type_size_bytes(inst->dst.type)); const unsigned src_byte_stride = required_src_byte_stride(devinfo, inst, i); @@ -650,9 +648,16 @@ namespace { subscript(inst->dst, raw_type, j)); } - for (unsigned j = 0; j < n; j++) - ibld.at(block, inst->next).MOV(subscript(inst->dst, raw_type, j), - subscript(tmp, raw_type, j)); + for (unsigned j = 0; j < n; j++) { + fs_inst *jnst = ibld.at(block, inst->next).MOV(subscript(inst->dst, raw_type, j), + subscript(tmp, raw_type, j)); + if (has_subdword_integer_region_restriction(v->devinfo, jnst)) { + /* The copy isn't guaranteed to comply with all subdword integer + * regioning restrictions in some cases. Lower it recursively. + */ + lower_instruction(v, block, jnst); + } + } /* If the destination was an accumulator, after lowering it will be a * GRF. Clear writes_accumulator for the instruction. diff --git a/src/intel/compiler/brw_ir_fs.h b/src/intel/compiler/brw_ir_fs.h index b0120178503..997927c9ae9 100644 --- a/src/intel/compiler/brw_ir_fs.h +++ b/src/intel/compiler/brw_ir_fs.h @@ -445,8 +445,12 @@ has_subdword_integer_region_restriction(const intel_device_info *devinfo, brw_type_size_bytes(inst->dst.type)) < 4) { for (unsigned i = 0; i < num_srcs; i++) { if (brw_type_is_int(srcs[i].type) && - brw_type_size_bytes(srcs[i].type) < 4 && - byte_stride(srcs[i]) >= 4) + ((brw_type_size_bytes(srcs[i].type) < 4 && + byte_stride(srcs[i]) >= 4) || + (MAX2(byte_stride(inst->dst), + brw_type_size_bytes(inst->dst.type)) == 1 && + brw_type_size_bytes(srcs[i].type) == 1 && + byte_stride(srcs[i]) >= 2))) return true; } }