[Mesa-dev] [PATCH 03/10] intel/fs: Fix bug in lower_simd_width while splitting an instruction which was already split.

Francisco Jerez currojerez at riseup.net
Mon Jan 7 19:58:39 UTC 2019


Iago Toral <itoral at igalia.com> writes:

> On Sat, 2018-12-29 at 12:38 -0800, Francisco Jerez wrote:
>> This seems to be a problem in combination with the lower_regioning
>> pass introduced by a future commit, which can modify a SIMD-split
>> instruction causing its execution size to become illegal again.  A
>> subsequent call to lower_simd_width() would hit this bug on a future
>> platform.
>> 
>> Cc: mesa-stable at lists.freedesktop.org
>> ---
>>  src/intel/compiler/brw_fs.cpp | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>> 
>> diff --git a/src/intel/compiler/brw_fs.cpp
>> b/src/intel/compiler/brw_fs.cpp
>> index 97544fdf465..4aacc72a1b7 100644
>> --- a/src/intel/compiler/brw_fs.cpp
>> +++ b/src/intel/compiler/brw_fs.cpp
>> @@ -5666,7 +5666,7 @@ static fs_reg
>>  emit_unzip(const fs_builder &lbld, fs_inst *inst, unsigned i)
>>  {
>>     /* Specified channel group from the source region. */
>> -   const fs_reg src = horiz_offset(inst->src[i], lbld.group());
>> +   const fs_reg src = horiz_offset(inst->src[i], lbld.group() -
>> inst->group);
>
> Should we assert that lbld.group >= inst->group? Same below.
>

The IR will fail validation anytime that's not the case.  But I can add
the assertions in both places if that makes you feel more comfortable.

>>     if (needs_src_copy(lbld, inst, i)) {
>>        /* Builder of the right width to perform the copy avoiding
>> uninitialized
>> @@ -5757,7 +5757,7 @@ emit_zip(const fs_builder &lbld_before, const
>> fs_builder &lbld_after,
>>     assert(lbld_before.group() == lbld_after.group());
>>  
>>     /* Specified channel group from the destination region. */
>> -   const fs_reg dst = horiz_offset(inst->dst, lbld_after.group());
>> +   const fs_reg dst = horiz_offset(inst->dst, lbld_after.group() -
>> inst->group);
>>     const unsigned dst_size = inst->size_written /
>>        inst->dst.component_size(inst->exec_size);
>>  
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 227 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20190107/539f6964/attachment.sig>


More information about the mesa-dev mailing list