[Mesa-dev] [PATCH 12/59] i965: add brw_imm_df

[Samuel Iglesias Gonsálvez]

On 11/05/16 05:56, Francisco Jerez wrote:
> Samuel Iglesias Gonsálvez <siglesias at igalia.com> writes:
> 
>> From: Connor Abbott <connor.w.abbott at intel.com>
>>
>> v2 (Iago)
>>   - Fixup accessibility in backend_reg
>>
>> Signed-off-by: Iago Toral Quiroga <itoral at igalia.com>
> 
> I've just noticed (while running valgrind) that this patch causes
> serious breakage in the back-end.  The reason is that the extra bits
> required to make room for the df field of the union don't get
> initialized in all codepaths, so backend_reg comparisons done using
> memcmp() can basically return random results now.  Can you please look
> into this?  Some ways to fix it would be to make sure we zero-initialize
> the whole brw_reg in all cases (or at least the union padding), or stop
> using memcmp() to compare registers -- I guess the latter might be
> somewhat less intrusive and increase the likelihood that we can get this
> sorted out timely.
> 

Attached is a patch for it, I initialized all union bits to zero before
setting them in brw_reg(). Can you test it? If it is not fixed, Would
you mind sending me an example to run it with valgrind here?

I am thinking that maybe we want to change backend_reg::equals() if this
doesn't work.

Sam

>> ---
>>  src/mesa/drivers/dri/i965/brw_reg.h    | 9 +++++++++
>>  src/mesa/drivers/dri/i965/brw_shader.h | 1 +
>>  2 files changed, 10 insertions(+)
>>
>> diff --git a/src/mesa/drivers/dri/i965/brw_reg.h b/src/mesa/drivers/dri/i965/brw_reg.h
>> index b84c709..6d51623 100644
>> --- a/src/mesa/drivers/dri/i965/brw_reg.h
>> +++ b/src/mesa/drivers/dri/i965/brw_reg.h
>> @@ -254,6 +254,7 @@ struct brw_reg {
>>           unsigned pad1:1;
>>        };
>>  
>> +      double df;
>>        float f;
>>        int   d;
>>        unsigned ud;
>> @@ -544,6 +545,14 @@ brw_imm_reg(enum brw_reg_type type)
>>  
>>  /** Construct float immediate register */
>>  static inline struct brw_reg
>> +brw_imm_df(double df)
>> +{
>> +   struct brw_reg imm = brw_imm_reg(BRW_REGISTER_TYPE_DF);
>> +   imm.df = df;
>> +   return imm;
>> +}
>> +
>> +static inline struct brw_reg
>>  brw_imm_f(float f)
>>  {
>>     struct brw_reg imm = brw_imm_reg(BRW_REGISTER_TYPE_F);
>> diff --git a/src/mesa/drivers/dri/i965/brw_shader.h b/src/mesa/drivers/dri/i965/brw_shader.h
>> index fc228f6..f6f6167 100644
>> --- a/src/mesa/drivers/dri/i965/brw_shader.h
>> +++ b/src/mesa/drivers/dri/i965/brw_shader.h
>> @@ -90,6 +90,7 @@ struct backend_reg : private brw_reg
>>     using brw_reg::width;
>>     using brw_reg::hstride;
>>  
>> +   using brw_reg::df;
>>     using brw_reg::f;
>>     using brw_reg::d;
>>     using brw_reg::ud;
>> -- 
>> 2.5.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: 0001-i965-initialize-struct-brw_reg-s-union-bits-to-zero.patch
Type: text/x-patch
Size: 1426 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20160511/47586603/attachment-0001.bin>