[Mesa-dev] [PATCH v3 15/24] i965/vec4: fix VEC4_OPCODE_FROM_DOUBLE for IVB/BYT

[Samuel Iglesias Gonsálvez]

On 04/03/17 00:55, Francisco Jerez wrote:
> Samuel Iglesias Gonsálvez <siglesias at igalia.com> writes:
> 
>> From: "Juan A. Suarez Romero" <jasuarez at igalia.com>
>>
>> In the generator we must generate slightly different code for
>> Ivybridge/Baytrail, because of the way the stride works in
>> this hardware.
>> ---
>>  src/mesa/drivers/dri/i965/brw_vec4_generator.cpp | 17 ++++++++++++++++-
>>  1 file changed, 16 insertions(+), 1 deletion(-)
>>
>> diff --git a/src/mesa/drivers/dri/i965/brw_vec4_generator.cpp b/src/mesa/drivers/dri/i965/brw_vec4_generator.cpp
>> index 7bb1ab1879c..7fa1afc9073 100644
>> --- a/src/mesa/drivers/dri/i965/brw_vec4_generator.cpp
>> +++ b/src/mesa/drivers/dri/i965/brw_vec4_generator.cpp
>> @@ -1948,13 +1948,28 @@ generate_code(struct brw_codegen *p,
>>  
>>           brw_set_default_access_mode(p, BRW_ALIGN_1);
>>  
>> -         dst.hstride = BRW_HORIZONTAL_STRIDE_2;
> 
> How would this know whether there was enough space allocated in the
> destination to hold the destination value with a stride of two?  What if
> the following SIMD vector in the same GRF allocation actually contained
> useful single-precision data that is getting corrupted by the oversized
> strided destination?  I think we should be doing this pre-regalloc so we
> can allocate a temporary large enough to hold the strided value...
> 

When we emit the instruction in emit_conversion_from_double(), we
already allocated space for a dvec4, retype it to vec4 and mark
size_written to 2 GRFs. This means, in practice, than we have already
enough space allocated to hold the destination value with a stride of two.

The problem seen by patch 16 is that the split needs a temporary
register to save the value because we are re-using the same dst reg to
write the spread value and then shrink it. So we do it in a temporary
register and then save the result in the real destination at the
corresponding horizontal offset.

We cannot use the real destination in the split instruction because we
would end up having a shrinking mov like this when generating the second
split instruction:

mov(8)          g20.4<1>F       g20.4<8,4,2>F

which is not valid because we only VertStride must be used to cross GRF
register boundaries, hence the need of the temporary.


>> +         /* When converting from DF->F, we set destination's stride as 2 as an
>> +          * aligment requirement. But in IVB/BYT, each DF implicitly writes
>> +          * two floats, being the first one the converted value. So we don't
>> +          * need to explicitly set stride 2, but 1.
>> +          */
>> +         if (devinfo->gen == 7 && !devinfo->is_haswell)
>> +            dst.hstride = BRW_HORIZONTAL_STRIDE_1;
>> +         else
>> +            dst.hstride = BRW_HORIZONTAL_STRIDE_2;
>> +
>>           dst.width = BRW_WIDTH_4;
>>           src[0].vstride = BRW_VERTICAL_STRIDE_4;
>>           src[0].width = BRW_WIDTH_4;
>>           brw_MOV(p, dst, src[0]);
>>  
> spread(dst, desired-stride) so you don't mess up the original
> destination brw_reg and have to fix it up again later.
> 

spread() doesn't set but add 'desired-stride' value to both register's
vertical and horizontal stride. I am going to use stride() which is the
one we want.

Sam

>>           struct brw_reg dst_as_src = dst;
>> +         /* As we have set horizontal stride 1 instead of 2 in IVB/BYT, we
>> +          * need to fix it here to have the expected value.
>> +          */
>> +         if (devinfo->gen == 7 && !devinfo->is_haswell)
>> +            dst_as_src.hstride = BRW_HORIZONTAL_STRIDE_2;
>> +
>>           dst.hstride = BRW_HORIZONTAL_STRIDE_1;
>>           dst.width = BRW_WIDTH_8;
>>           brw_MOV(p, dst, dst_as_src);
>> -- 
>> 2.11.0
>>
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev





-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 862 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20170307/59369dd0/attachment.sig>