On 11/05/16 05:56, Francisco Jerez wrote:
> Samuel Iglesias Gonsálvez <[email protected]> writes:
>
>> From: Connor Abbott <[email protected]>
>>
>> v2 (Iago)
>> - Fixup accessibility in backend_reg
>>
>> Signed-off-by: Iago Toral Quiroga <[email protected]>
>
> 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
>> [email protected]
>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>From 35254624d63b77aa2024bc2b08612e28cae4bb98 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Samuel=20Iglesias=20Gons=C3=A1lvez?= <[email protected]>
Date: Wed, 11 May 2016 07:44:10 +0200
Subject: [PATCH] i965: initialize struct brw_reg's union bits to zero.
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
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.
Initialize them to zero before setting the rest of union's fields.
Signed-off-by: Samuel Iglesias Gonsálvez <[email protected]>
Reported-by: Francisco Jerez <[email protected]>
---
src/mesa/drivers/dri/i965/brw_reg.h | 3 +++
1 file changed, 3 insertions(+)
diff --git a/src/mesa/drivers/dri/i965/brw_reg.h b/src/mesa/drivers/dri/i965/brw_reg.h
index 6d51623..3b76d7d 100644
--- a/src/mesa/drivers/dri/i965/brw_reg.h
+++ b/src/mesa/drivers/dri/i965/brw_reg.h
@@ -338,6 +338,9 @@ brw_reg(enum brw_reg_file file,
reg.subnr = subnr * type_sz(type);
reg.nr = nr;
+ /* Initialize all union's bits to zero before setting them. */
+ reg.df = 0;
+
/* Could do better: If the reg is r5.3<0;1,0>, we probably want to
* set swizzle and writemask to W, as the lower bits of subnr will
* be lost when converted to align16. This is probably too much to
--
2.5.0
_______________________________________________
mesa-dev mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
