On Wed, Apr 8, 2015 at 4:38 PM, Ian Romanick <i...@freedesktop.org> wrote:
> From: Ian Romanick <ian.d.roman...@intel.com>
>
> This method is similar to the existing ::equals method. Instead of
> testing that two src_regs are equal to each other, it tests that one is
> the negation of the other.
>
> Signed-off-by: Ian Romanick <ian.d.roman...@intel.com>
> ---
> src/mesa/drivers/dri/i965/brw_ir_vec4.h | 1 +
> src/mesa/drivers/dri/i965/brw_vec4.cpp | 43
> +++++++++++++++++++++++++++++++++
> 2 files changed, 44 insertions(+)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_ir_vec4.h
> b/src/mesa/drivers/dri/i965/brw_ir_vec4.h
> index d3bd64d..449795a 100644
> --- a/src/mesa/drivers/dri/i965/brw_ir_vec4.h
> +++ b/src/mesa/drivers/dri/i965/brw_ir_vec4.h
> @@ -49,6 +49,7 @@ public:
> src_reg(struct brw_reg reg);
>
> bool equals(const src_reg &r) const;
> + bool negative_equals(const src_reg &r) const;
>
> src_reg(class vec4_visitor *v, const struct glsl_type *type);
> src_reg(class vec4_visitor *v, const struct glsl_type *type, int size);
> diff --git a/src/mesa/drivers/dri/i965/brw_vec4.cpp
> b/src/mesa/drivers/dri/i965/brw_vec4.cpp
> index ef2fd40..d5286c2 100644
> --- a/src/mesa/drivers/dri/i965/brw_vec4.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_vec4.cpp
> @@ -328,6 +328,49 @@ src_reg::equals(const src_reg &r) const
> }
>
> bool
> +src_reg::negative_equals(const src_reg &r) const
> +{
> + if (file != r.file)
> + return false;
> +
> + if (file == IMM) {
> + if (!(reg == r.reg &&
> + reg_offset == r.reg_offset &&
> + type == r.type &&
> + negate == r.negate &&
> + abs == r.abs &&
> + swizzle == r.swizzle &&
> + !reladdr && !r.reladdr))
I would have applied DeMorgan's Law and made this a series of ORs.
> + return false;
> +
> + switch (fixed_hw_reg.type) {
I don't think we keep fixed_hw_reg.type in sync with src_reg's type.
(Another reason to move towards a brw_reg-based register model)
Looking at the constructors for immediate, this appears to be the
case. Just changing this to switch (type) should do the trick.
> + case BRW_REGISTER_TYPE_F:
> + return memcmp(&fixed_hw_reg, &r.fixed_hw_reg,
> + sizeof(fixed_hw_reg) - sizeof(fixed_hw_reg.dw1)) == 0
> &&
> + fixed_hw_reg.dw1.f == -r.fixed_hw_reg.dw1.f;
I don't think you need the memcmp. For immediates, we really just use
the fixed_hw_reg for the immediate storage itself and none of the
other metadata.
> +
> + case BRW_REGISTER_TYPE_D:
> + return memcmp(&fixed_hw_reg, &r.fixed_hw_reg,
> + sizeof(fixed_hw_reg) - sizeof(fixed_hw_reg.dw1)) == 0
> &&
> + fixed_hw_reg.dw1.d == -r.fixed_hw_reg.dw1.d;
> +
> + default:
> + return false;
> + }
> + } else {
> + return reg == r.reg &&
> + reg_offset == r.reg_offset &&
> + type == r.type &&
> + negate != r.negate &&
> + abs == r.abs &&
> + swizzle == r.swizzle &&
> + !reladdr && !r.reladdr &&
> + memcmp(&fixed_hw_reg, &r.fixed_hw_reg,
> + sizeof(fixed_hw_reg)) == 0;
Indent these to match reg == ...
I think the function does what it advertises. I just don't think that
happens to be the thing you really want. See next patch's reply.
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
