-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA256
On 13/05/16 05:38, Francisco Jerez wrote: > Samuel Iglesias Gonsálvez <[email protected]> writes: > >> 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. Check field by field to avoid this. >> >> Signed-off-by: Samuel Iglesias Gonsálvez <[email protected]> >> Reported-by: Francisco Jerez <[email protected]> > > This seemed to avoid the problem on at least the few tests I ran > manually under valgrind, but I have doubts that it is a complete > fix, grepping for 'memcmp' in the back-end gives the following uses > with a brw_reg as argument that will likely still give > non-deterministic results even with this patch applied: > > brw_fs_generator.cpp:1009: if (memcmp(&surface_reg, > &sampler_reg, sizeof(surface_reg)) == 0) { > brw_vec4_generator.cpp:298: if (memcmp(&surface_reg, > &sampler_reg, sizeof(surface_reg)) == 0) { > > Might be a good idea to factor out the manual comparison into a > static function in brw_reg.h you could use in the FS and VEC4 > generators to avoid duplicating the complex expression? Or maybe > find out why PATCH 1 is not sufficient to fix the issue? Most > likely something else other than brw_reg() is attempting to > initialize brw_reg structs manually? > OK. I am looking into it today. Thanks for the suggestions and comments. Sam > One more comment below. > >> --- src/mesa/drivers/dri/i965/brw_reg.h | 1 + >> src/mesa/drivers/dri/i965/brw_shader.cpp | 21 >> +++++++++++++++++++-- 2 files changed, 20 insertions(+), 2 >> deletions(-) >> >> diff --git a/src/mesa/drivers/dri/i965/brw_reg.h >> b/src/mesa/drivers/dri/i965/brw_reg.h index 3b76d7d..68128bd >> 100644 --- a/src/mesa/drivers/dri/i965/brw_reg.h +++ >> b/src/mesa/drivers/dri/i965/brw_reg.h @@ -233,6 +233,7 @@ >> uint32_t brw_swizzle_immediate(enum brw_reg_type type, uint32_t >> x, unsigned swz) * WM programs to implement shaders decomposed >> into "channel serial" * or "structure of array" form: */ +/* >> IMPORTANT: update backend_reg::equals() if you add a new field >> here. */ struct brw_reg { enum brw_reg_type type:4; enum >> brw_reg_file file:3; /* :2 hardware format */ diff --git >> a/src/mesa/drivers/dri/i965/brw_shader.cpp >> b/src/mesa/drivers/dri/i965/brw_shader.cpp index a23f14e..7871f51 >> 100644 --- a/src/mesa/drivers/dri/i965/brw_shader.cpp +++ >> b/src/mesa/drivers/dri/i965/brw_shader.cpp @@ -687,8 +687,25 @@ >> backend_shader::backend_shader(const struct brw_compiler >> *compiler, bool backend_reg::equals(const backend_reg &r) const >> { - return memcmp((brw_reg *)this, (brw_reg *)&r, >> sizeof(brw_reg)) == 0 && - reg_offset == r.reg_offset; + >> bool equal = true; > > No need for this variable, you can just return false early. > OK. >> + + if (this->reg_offset != r.reg_offset || + this->type >> != r.type || + this->file != r.file || + this->negate >> != r.negate || + this->abs != r.abs || + >> this->address_mode != r.address_mode || + this->pad0 != >> r.pad0 || + this->subnr != r.subnr || + this->nr != >> r.nr || + this->ud != r.ud) + equal = false; + + /* >> Check higher 32 bits of the brw_reg's union if they are filled >> */ + if (equal && this->type == BRW_REGISTER_TYPE_DF && >> this->df != r.df) + equal = false; + + return equal; } >> >> bool -- 2.5.0 >> >> _______________________________________________ mesa-dev mailing >> list [email protected] >> https://lists.freedesktop.org/mailman/listinfo/mesa-dev -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQIcBAEBCAAGBQJXNWJRAAoJEH/0ujLxfcND0lsQAMQQiEQ9zeo06iPPE+xHNHdY MpZLE+4+pWsBS5Q0PnQPZ9rJMOJQmNI55aA/htUH4tOmShyg8q3tSI4PIZzdSrrS 0XjQCMZI7O43Sca/aBG633TFrqxDBEr/2JKpjXXm//ZRvnJ8pTfVtro/ams/pAed jOC1gNs/qfIVlHnQMZWOAViKLSt9oG0jNsBnsvp3VAlnveq/RaHxApPGhHKosbbi x6WjmNI5UihEiyJqOuaCVzIejeKJFOBKEORPybi6YLRwjwvbg7PVYDdSWixuVsLF vz1X2VCQRUw/4uck9uqhzFk37afkftU8RamjYyXLfhOZ2F+kc1+QVFlf5ptUvXiz cmUZGIwvLHz9y1USNzLuIiIhHbGV+Qb2oMeo2pkK/teHsIeDJbfEaCTrDeGbLQmH qdboQtRR1XwMKcDm8qBvrqG7aQ9N0cLMQH4Cfk1mApOZovNkdEa7CjYkBGDvshUt VWLjq0v2lYUsgKtHYB4r8GXxdlSEXhK312VrzFoDOIwVfVahQupDOKCNEcLZj2jC ngLyoFsCPg1YnyFLXnAlgcvbyqKAUev6kSZHogSaeojQJZJlnfJMqZi2IMAUvlt+ GQSjsMzHXh/kKQkVIreMQNaBDWaXgIfbzDBiNHcYVvrNti94zt/p8r3J7DU+McHw +NhNlvEn/rd177sJRtPB =VZah -----END PGP SIGNATURE----- _______________________________________________ mesa-dev mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/mesa-dev
