Kenneth Graunke <kenn...@whitecape.org> writes:
> Commit 5310bca024f77da40ea6f4c275455f9cb0528f9e added a new "double df"
> field to the brw_reg struct, adding an extra 4 bytes of data that isn't
> usually initialized (or may contain irrelevant garbage if the struct is
> mutated). This means that it's no longer safe to memcmp().
>
> Instead, add a brw_regs_equal() function which ignores the extra df bits
> unless they matter. To keep the implementation cheap, we wrap the first
> set of fields in a union/struct so that we can use a single DWord
> comparison.
>
> Signed-off-by: Kenneth Graunke <kenn...@whitecape.org>
> ---
> src/mesa/drivers/dri/i965/brw_fs_generator.cpp | 2 +-
> src/mesa/drivers/dri/i965/brw_reg.h | 27
> +++++++++++++++++-------
> src/mesa/drivers/dri/i965/brw_shader.cpp | 2 +-
> src/mesa/drivers/dri/i965/brw_vec4_generator.cpp | 2 +-
> 4 files changed, 22 insertions(+), 11 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_fs_generator.cpp
> b/src/mesa/drivers/dri/i965/brw_fs_generator.cpp
> index 4f6f3a3..3b50a82 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs_generator.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs_generator.cpp
> @@ -1010,7 +1010,7 @@ fs_generator::generate_tex(fs_inst *inst, struct
> brw_reg dst, struct brw_reg src
> brw_set_default_mask_control(p, BRW_MASK_DISABLE);
> brw_set_default_access_mode(p, BRW_ALIGN_1);
>
> - if (memcmp(&surface_reg, &sampler_reg, sizeof(surface_reg)) == 0) {
> + if (brw_regs_equal(&surface_reg, &sampler_reg)) {
> brw_MUL(p, addr, sampler_reg, brw_imm_uw(0x101));
> } else {
> brw_SHL(p, addr, sampler_reg, brw_imm_ud(8));
> diff --git a/src/mesa/drivers/dri/i965/brw_reg.h
> b/src/mesa/drivers/dri/i965/brw_reg.h
> index 6d51623..71e1024 100644
> --- a/src/mesa/drivers/dri/i965/brw_reg.h
> +++ b/src/mesa/drivers/dri/i965/brw_reg.h
> @@ -234,14 +234,19 @@ uint32_t brw_swizzle_immediate(enum brw_reg_type type,
> uint32_t x, unsigned swz)
> * or "structure of array" form:
> */
> struct brw_reg {
> - enum brw_reg_type type:4;
> - enum brw_reg_file file:3; /* :2 hardware format */
> - unsigned negate:1; /* source only */
> - unsigned abs:1; /* source only */
> - unsigned address_mode:1; /* relative addressing, hopefully! */
> - unsigned pad0:1;
> - unsigned subnr:5; /* :1 in align16 */
> - unsigned nr:16;
> + union {
> + struct {
> + enum brw_reg_type type:4;
> + enum brw_reg_file file:3; /* :2 hardware format */
> + unsigned negate:1; /* source only */
> + unsigned abs:1; /* source only */
> + unsigned address_mode:1; /* relative addressing, hopefully! */
> + unsigned pad0:1;
> + unsigned subnr:5; /* :1 in align16 */
> + unsigned nr:16;
> + };
> + uint32_t bits;
> + };
>
> union {
> struct {
> @@ -261,6 +266,12 @@ struct brw_reg {
> };
> };
>
> +static inline bool
> +brw_regs_equal(const struct brw_reg *a, const struct brw_reg *b)
> +{
> + const bool df = a->type == BRW_REGISTER_TYPE_DF && a->file == IMM;
> + return a->bits == b->bits && (df ? a->df == b->df : a->ud == b->ud);
> +}
>
> struct brw_indirect {
> unsigned addr_subnr:4;
> diff --git a/src/mesa/drivers/dri/i965/brw_shader.cpp
> b/src/mesa/drivers/dri/i965/brw_shader.cpp
> index a23f14e..8d9e309 100644
> --- a/src/mesa/drivers/dri/i965/brw_shader.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_shader.cpp
> @@ -687,7 +687,7 @@ 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 &&
> + return brw_regs_equal((brw_reg *)this, (brw_reg *)&r) &&
These casts should be redundant now, if the upcast is safe the compiler
will be able to figure it out for you. With that cleaned up:
Reviewed-by: Francisco Jerez <curroje...@riseup.net>
> reg_offset == r.reg_offset;
> }
>
> diff --git a/src/mesa/drivers/dri/i965/brw_vec4_generator.cpp
> b/src/mesa/drivers/dri/i965/brw_vec4_generator.cpp
> index 4b44c3a..baf4422 100644
> --- a/src/mesa/drivers/dri/i965/brw_vec4_generator.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_vec4_generator.cpp
> @@ -295,7 +295,7 @@ generate_tex(struct brw_codegen *p,
> brw_set_default_mask_control(p, BRW_MASK_DISABLE);
> brw_set_default_access_mode(p, BRW_ALIGN_1);
>
> - if (memcmp(&surface_reg, &sampler_reg, sizeof(surface_reg)) == 0) {
> + if (brw_regs_equal(&surface_reg, &sampler_reg)) {
> brw_MUL(p, addr, sampler_reg, brw_imm_uw(0x101));
> } else {
> brw_SHL(p, addr, sampler_reg, brw_imm_ud(8));
> --
> 2.8.2
signature.asc
Description: PGP signature
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
