Re: [Mesa-dev] [PATCH 1/6] i965: Add negative_equals methods

[Ian Romanick]

On 03/23/2018 12:17 PM, Chema Casanova wrote:
> 
> 
> On 23/03/18 19:27, Matt Turner wrote:
>> On Wed, Mar 21, 2018 at 5:58 PM, Ian Romanick  wrote:
>>> From: Ian Romanick 
>>>
>>> This method is similar to the existing ::equals methods.  Instead of
>>> testing that two src_regs are equal to each other, it tests that one is
>>> the negation of the other.
>>>
>>> v2: Simplify various checks based on suggestions from Matt.  Use
>>> src_reg::type instead of fixed_hw_reg.type in a check.  Also suggested
>>> by Matt.
>>>
>>> v3: Rebase on 3 years.  Fix some problems with negative_equals with VF
>>> constants.  Add fs_reg::negative_equals.
>>>
>>> Signed-off-by: Ian Romanick 
>>> ---
>>>  src/intel/compiler/brw_fs.cpp |  7 ++
>>>  src/intel/compiler/brw_ir_fs.h|  1 +
>>>  src/intel/compiler/brw_ir_vec4.h  |  1 +
>>>  src/intel/compiler/brw_reg.h  | 46 
>>> +++
>>>  src/intel/compiler/brw_shader.cpp |  6 +
>>>  src/intel/compiler/brw_shader.h   |  1 +
>>>  src/intel/compiler/brw_vec4.cpp   |  7 ++
>>>  7 files changed, 69 insertions(+)
>>>
>>> diff --git a/src/intel/compiler/brw_fs.cpp b/src/intel/compiler/brw_fs.cpp
>>> index 6eea532..3d454c3 100644
>>> --- a/src/intel/compiler/brw_fs.cpp
>>> +++ b/src/intel/compiler/brw_fs.cpp
>>> @@ -454,6 +454,13 @@ fs_reg::equals(const fs_reg ) const
>>>  }
>>>
>>>  bool
>>> +fs_reg::negative_equals(const fs_reg ) const
>>> +{
>>> +   return (this->backend_reg::negative_equals(r) &&
>>> +   stride == r.stride);
>>> +}
>>> +
>>> +bool
>>>  fs_reg::is_contiguous() const
>>>  {
>>> return stride == 1;
>>> diff --git a/src/intel/compiler/brw_ir_fs.h b/src/intel/compiler/brw_ir_fs.h
>>> index 54797ff..f06a33c 100644
>>> --- a/src/intel/compiler/brw_ir_fs.h
>>> +++ b/src/intel/compiler/brw_ir_fs.h
>>> @@ -41,6 +41,7 @@ public:
>>> fs_reg(enum brw_reg_file file, int nr, enum brw_reg_type type);
>>>
>>> bool equals(const fs_reg ) const;
>>> +   bool negative_equals(const fs_reg ) const;
>>> bool is_contiguous() const;
>>>
>>> /**
>>> diff --git a/src/intel/compiler/brw_ir_vec4.h 
>>> b/src/intel/compiler/brw_ir_vec4.h
>>> index cbaff2f..95c5119 100644
>>> --- a/src/intel/compiler/brw_ir_vec4.h
>>> +++ b/src/intel/compiler/brw_ir_vec4.h
>>> @@ -43,6 +43,7 @@ public:
>>> src_reg(struct ::brw_reg reg);
>>>
>>> bool equals(const src_reg ) const;
>>> +   bool negative_equals(const src_reg ) 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/intel/compiler/brw_reg.h b/src/intel/compiler/brw_reg.h
>>> index 7ad144b..732bddf 100644
>>> --- a/src/intel/compiler/brw_reg.h
>>> +++ b/src/intel/compiler/brw_reg.h
>>> @@ -255,6 +255,52 @@ brw_regs_equal(const struct brw_reg *a, const struct 
>>> brw_reg *b)
>>> return a->bits == b->bits && (df ? a->u64 == b->u64 : a->ud == b->ud);
>>>  }
>>>
>>> +static inline bool
>>> +brw_regs_negative_equal(const struct brw_reg *a, const struct brw_reg *b)
>>> +{
>>> +   if (a->file == IMM) {
>>> +  if (a->bits != b->bits)
>>> + return false;
>>> +
>>> +  switch (a->type) {
>>> +  case BRW_REGISTER_TYPE_UQ:
>>> +  case BRW_REGISTER_TYPE_Q:
>>> + return a->d64 == -b->d64;
>>> +  case BRW_REGISTER_TYPE_DF:
>>> + return a->df == -b->df;
>>> +  case BRW_REGISTER_TYPE_UD:
>>> +  case BRW_REGISTER_TYPE_D:
>>> + return a->d == -b->d;
>>> +  case BRW_REGISTER_TYPE_F:
>>> + return a->f == -b->f;
>>> +  case BRW_REGISTER_TYPE_VF:
>>> + /* It is tempting to treat 0 as a negation of 0 (and -0 as a 
>>> negation
>>> +  * of -0).  There are occasions where 0 or -0 is used and the 
>>> exact
>>> +  * bit pattern is desired.  At the very least, changing this to 
>>> allow
>>> +  * 0 as a negation of 0 causes some fp64 tests to fail on IVB.
>>> +  */
>>> + return a->ud == (b->ud ^ 0x80808080);
>>> +  case BRW_REGISTER_TYPE_UW:
>>> +  case BRW_REGISTER_TYPE_W:
>>> +  case BRW_REGISTER_TYPE_UV:
>>> +  case BRW_REGISTER_TYPE_V:
>>> +  case BRW_REGISTER_TYPE_HF:
>>> +  case BRW_REGISTER_TYPE_UB:
>>> +  case BRW_REGISTER_TYPE_B:
>>
>> There are no B/UB immediates, so you can move these to default. In
>> fact, I'd get rid of the default so we'll get a warning if there are
>> unhandled types. Probably the only one not already in the list is NF,
>> which should also be unreachable.
> 
>> Returning false for unimplemented types seems fine. Immediates of
>> those types are sufficiently rare that I don't expect this to catch
>> anything, and in the rare occurrence that it does I wouldn't want the
>> compiler to assert fail or do something undefined. Really I only
>> expect HF to ever get hit, and only after we actually start using it.
> 

Re: [Mesa-dev] [PATCH 1/6] i965: Add negative_equals methods

[Chema Casanova]

On 23/03/18 19:27, Matt Turner wrote:
> On Wed, Mar 21, 2018 at 5:58 PM, Ian Romanick  wrote:
>> From: Ian Romanick 
>>
>> This method is similar to the existing ::equals methods.  Instead of
>> testing that two src_regs are equal to each other, it tests that one is
>> the negation of the other.
>>
>> v2: Simplify various checks based on suggestions from Matt.  Use
>> src_reg::type instead of fixed_hw_reg.type in a check.  Also suggested
>> by Matt.
>>
>> v3: Rebase on 3 years.  Fix some problems with negative_equals with VF
>> constants.  Add fs_reg::negative_equals.
>>
>> Signed-off-by: Ian Romanick 
>> ---
>>  src/intel/compiler/brw_fs.cpp |  7 ++
>>  src/intel/compiler/brw_ir_fs.h|  1 +
>>  src/intel/compiler/brw_ir_vec4.h  |  1 +
>>  src/intel/compiler/brw_reg.h  | 46 
>> +++
>>  src/intel/compiler/brw_shader.cpp |  6 +
>>  src/intel/compiler/brw_shader.h   |  1 +
>>  src/intel/compiler/brw_vec4.cpp   |  7 ++
>>  7 files changed, 69 insertions(+)
>>
>> diff --git a/src/intel/compiler/brw_fs.cpp b/src/intel/compiler/brw_fs.cpp
>> index 6eea532..3d454c3 100644
>> --- a/src/intel/compiler/brw_fs.cpp
>> +++ b/src/intel/compiler/brw_fs.cpp
>> @@ -454,6 +454,13 @@ fs_reg::equals(const fs_reg ) const
>>  }
>>
>>  bool
>> +fs_reg::negative_equals(const fs_reg ) const
>> +{
>> +   return (this->backend_reg::negative_equals(r) &&
>> +   stride == r.stride);
>> +}
>> +
>> +bool
>>  fs_reg::is_contiguous() const
>>  {
>> return stride == 1;
>> diff --git a/src/intel/compiler/brw_ir_fs.h b/src/intel/compiler/brw_ir_fs.h
>> index 54797ff..f06a33c 100644
>> --- a/src/intel/compiler/brw_ir_fs.h
>> +++ b/src/intel/compiler/brw_ir_fs.h
>> @@ -41,6 +41,7 @@ public:
>> fs_reg(enum brw_reg_file file, int nr, enum brw_reg_type type);
>>
>> bool equals(const fs_reg ) const;
>> +   bool negative_equals(const fs_reg ) const;
>> bool is_contiguous() const;
>>
>> /**
>> diff --git a/src/intel/compiler/brw_ir_vec4.h 
>> b/src/intel/compiler/brw_ir_vec4.h
>> index cbaff2f..95c5119 100644
>> --- a/src/intel/compiler/brw_ir_vec4.h
>> +++ b/src/intel/compiler/brw_ir_vec4.h
>> @@ -43,6 +43,7 @@ public:
>> src_reg(struct ::brw_reg reg);
>>
>> bool equals(const src_reg ) const;
>> +   bool negative_equals(const src_reg ) 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/intel/compiler/brw_reg.h b/src/intel/compiler/brw_reg.h
>> index 7ad144b..732bddf 100644
>> --- a/src/intel/compiler/brw_reg.h
>> +++ b/src/intel/compiler/brw_reg.h
>> @@ -255,6 +255,52 @@ brw_regs_equal(const struct brw_reg *a, const struct 
>> brw_reg *b)
>> return a->bits == b->bits && (df ? a->u64 == b->u64 : a->ud == b->ud);
>>  }
>>
>> +static inline bool
>> +brw_regs_negative_equal(const struct brw_reg *a, const struct brw_reg *b)
>> +{
>> +   if (a->file == IMM) {
>> +  if (a->bits != b->bits)
>> + return false;
>> +
>> +  switch (a->type) {
>> +  case BRW_REGISTER_TYPE_UQ:
>> +  case BRW_REGISTER_TYPE_Q:
>> + return a->d64 == -b->d64;
>> +  case BRW_REGISTER_TYPE_DF:
>> + return a->df == -b->df;
>> +  case BRW_REGISTER_TYPE_UD:
>> +  case BRW_REGISTER_TYPE_D:
>> + return a->d == -b->d;
>> +  case BRW_REGISTER_TYPE_F:
>> + return a->f == -b->f;
>> +  case BRW_REGISTER_TYPE_VF:
>> + /* It is tempting to treat 0 as a negation of 0 (and -0 as a 
>> negation
>> +  * of -0).  There are occasions where 0 or -0 is used and the exact
>> +  * bit pattern is desired.  At the very least, changing this to 
>> allow
>> +  * 0 as a negation of 0 causes some fp64 tests to fail on IVB.
>> +  */
>> + return a->ud == (b->ud ^ 0x80808080);
>> +  case BRW_REGISTER_TYPE_UW:
>> +  case BRW_REGISTER_TYPE_W:
>> +  case BRW_REGISTER_TYPE_UV:
>> +  case BRW_REGISTER_TYPE_V:
>> +  case BRW_REGISTER_TYPE_HF:
>> +  case BRW_REGISTER_TYPE_UB:
>> +  case BRW_REGISTER_TYPE_B:
> 
> There are no B/UB immediates, so you can move these to default. In
> fact, I'd get rid of the default so we'll get a warning if there are
> unhandled types. Probably the only one not already in the list is NF,
> which should also be unreachable.

> Returning false for unimplemented types seems fine. Immediates of
> those types are sufficiently rare that I don't expect this to catch
> anything, and in the rare occurrence that it does I wouldn't want the
> compiler to assert fail or do something undefined. Really I only
> expect HF to ever get hit, and only after we actually start using it.

According to PRM:

"For a word, unsigned word, or half-float immediate data, software
must replicate the same 16-bit immediate value to both the lower word
and the high word of the 

Re: [Mesa-dev] [PATCH 1/6] i965: Add negative_equals methods

[Matt Turner]

On Wed, Mar 21, 2018 at 5:58 PM, Ian Romanick  wrote:
> From: Ian Romanick 
>
> This method is similar to the existing ::equals methods.  Instead of
> testing that two src_regs are equal to each other, it tests that one is
> the negation of the other.
>
> v2: Simplify various checks based on suggestions from Matt.  Use
> src_reg::type instead of fixed_hw_reg.type in a check.  Also suggested
> by Matt.
>
> v3: Rebase on 3 years.  Fix some problems with negative_equals with VF
> constants.  Add fs_reg::negative_equals.
>
> Signed-off-by: Ian Romanick 
> ---
>  src/intel/compiler/brw_fs.cpp |  7 ++
>  src/intel/compiler/brw_ir_fs.h|  1 +
>  src/intel/compiler/brw_ir_vec4.h  |  1 +
>  src/intel/compiler/brw_reg.h  | 46 
> +++
>  src/intel/compiler/brw_shader.cpp |  6 +
>  src/intel/compiler/brw_shader.h   |  1 +
>  src/intel/compiler/brw_vec4.cpp   |  7 ++
>  7 files changed, 69 insertions(+)
>
> diff --git a/src/intel/compiler/brw_fs.cpp b/src/intel/compiler/brw_fs.cpp
> index 6eea532..3d454c3 100644
> --- a/src/intel/compiler/brw_fs.cpp
> +++ b/src/intel/compiler/brw_fs.cpp
> @@ -454,6 +454,13 @@ fs_reg::equals(const fs_reg ) const
>  }
>
>  bool
> +fs_reg::negative_equals(const fs_reg ) const
> +{
> +   return (this->backend_reg::negative_equals(r) &&
> +   stride == r.stride);
> +}
> +
> +bool
>  fs_reg::is_contiguous() const
>  {
> return stride == 1;
> diff --git a/src/intel/compiler/brw_ir_fs.h b/src/intel/compiler/brw_ir_fs.h
> index 54797ff..f06a33c 100644
> --- a/src/intel/compiler/brw_ir_fs.h
> +++ b/src/intel/compiler/brw_ir_fs.h
> @@ -41,6 +41,7 @@ public:
> fs_reg(enum brw_reg_file file, int nr, enum brw_reg_type type);
>
> bool equals(const fs_reg ) const;
> +   bool negative_equals(const fs_reg ) const;
> bool is_contiguous() const;
>
> /**
> diff --git a/src/intel/compiler/brw_ir_vec4.h 
> b/src/intel/compiler/brw_ir_vec4.h
> index cbaff2f..95c5119 100644
> --- a/src/intel/compiler/brw_ir_vec4.h
> +++ b/src/intel/compiler/brw_ir_vec4.h
> @@ -43,6 +43,7 @@ public:
> src_reg(struct ::brw_reg reg);
>
> bool equals(const src_reg ) const;
> +   bool negative_equals(const src_reg ) 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/intel/compiler/brw_reg.h b/src/intel/compiler/brw_reg.h
> index 7ad144b..732bddf 100644
> --- a/src/intel/compiler/brw_reg.h
> +++ b/src/intel/compiler/brw_reg.h
> @@ -255,6 +255,52 @@ brw_regs_equal(const struct brw_reg *a, const struct 
> brw_reg *b)
> return a->bits == b->bits && (df ? a->u64 == b->u64 : a->ud == b->ud);
>  }
>
> +static inline bool
> +brw_regs_negative_equal(const struct brw_reg *a, const struct brw_reg *b)
> +{
> +   if (a->file == IMM) {
> +  if (a->bits != b->bits)
> + return false;
> +
> +  switch (a->type) {
> +  case BRW_REGISTER_TYPE_UQ:
> +  case BRW_REGISTER_TYPE_Q:
> + return a->d64 == -b->d64;
> +  case BRW_REGISTER_TYPE_DF:
> + return a->df == -b->df;
> +  case BRW_REGISTER_TYPE_UD:
> +  case BRW_REGISTER_TYPE_D:
> + return a->d == -b->d;
> +  case BRW_REGISTER_TYPE_F:
> + return a->f == -b->f;
> +  case BRW_REGISTER_TYPE_VF:
> + /* It is tempting to treat 0 as a negation of 0 (and -0 as a 
> negation
> +  * of -0).  There are occasions where 0 or -0 is used and the exact
> +  * bit pattern is desired.  At the very least, changing this to 
> allow
> +  * 0 as a negation of 0 causes some fp64 tests to fail on IVB.
> +  */
> + return a->ud == (b->ud ^ 0x80808080);
> +  case BRW_REGISTER_TYPE_UW:
> +  case BRW_REGISTER_TYPE_W:
> +  case BRW_REGISTER_TYPE_UV:
> +  case BRW_REGISTER_TYPE_V:
> +  case BRW_REGISTER_TYPE_HF:
> +  case BRW_REGISTER_TYPE_UB:
> +  case BRW_REGISTER_TYPE_B:

There are no B/UB immediates, so you can move these to default. In
fact, I'd get rid of the default so we'll get a warning if there are
unhandled types. Probably the only one not already in the list is NF,
which should also be unreachable.

Returning false for unimplemented types seems fine. Immediates of
those types are sufficiently rare that I don't expect this to catch
anything, and in the rare occurrence that it does I wouldn't want the
compiler to assert fail or do something undefined. Really I only
expect HF to ever get hit, and only after we actually start using it.

Reviewed-by: Matt Turner 
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 1/6] i965: Add negative_equals methods

[Alejandro Piñeiro]

On 23/03/18 16:13, Ian Romanick wrote:
> On 03/23/2018 01:21 AM, Alejandro Piñeiro wrote:
>> On 22/03/18 19:08, Ian Romanick wrote:
>>> On 03/22/2018 01:12 AM, Alejandro Piñeiro wrote:
 Looks good in general, just a comment below.


 On 22/03/18 01:58, Ian Romanick wrote:
> From: Ian Romanick 
>
> This method is similar to the existing ::equals methods.  Instead of
> testing that two src_regs are equal to each other, it tests that one is
> the negation of the other.
>
> v2: Simplify various checks based on suggestions from Matt.  Use
> src_reg::type instead of fixed_hw_reg.type in a check.  Also suggested
> by Matt.
>
> v3: Rebase on 3 years.  Fix some problems with negative_equals with VF
> constants.  Add fs_reg::negative_equals.
>
> Signed-off-by: Ian Romanick 
> ---
>  src/intel/compiler/brw_fs.cpp |  7 ++
>  src/intel/compiler/brw_ir_fs.h|  1 +
>  src/intel/compiler/brw_ir_vec4.h  |  1 +
>  src/intel/compiler/brw_reg.h  | 46 
> +++
>  src/intel/compiler/brw_shader.cpp |  6 +
>  src/intel/compiler/brw_shader.h   |  1 +
>  src/intel/compiler/brw_vec4.cpp   |  7 ++
>  7 files changed, 69 insertions(+)
>
> diff --git a/src/intel/compiler/brw_fs.cpp b/src/intel/compiler/brw_fs.cpp
> index 6eea532..3d454c3 100644
> --- a/src/intel/compiler/brw_fs.cpp
> +++ b/src/intel/compiler/brw_fs.cpp
> @@ -454,6 +454,13 @@ fs_reg::equals(const fs_reg ) const
>  }
>  
>  bool
> +fs_reg::negative_equals(const fs_reg ) const
> +{
> +   return (this->backend_reg::negative_equals(r) &&
> +   stride == r.stride);
> +}
> +
> +bool
>  fs_reg::is_contiguous() const
>  {
> return stride == 1;
> diff --git a/src/intel/compiler/brw_ir_fs.h 
> b/src/intel/compiler/brw_ir_fs.h
> index 54797ff..f06a33c 100644
> --- a/src/intel/compiler/brw_ir_fs.h
> +++ b/src/intel/compiler/brw_ir_fs.h
> @@ -41,6 +41,7 @@ public:
> fs_reg(enum brw_reg_file file, int nr, enum brw_reg_type type);
>  
> bool equals(const fs_reg ) const;
> +   bool negative_equals(const fs_reg ) const;
> bool is_contiguous() const;
>  
> /**
> diff --git a/src/intel/compiler/brw_ir_vec4.h 
> b/src/intel/compiler/brw_ir_vec4.h
> index cbaff2f..95c5119 100644
> --- a/src/intel/compiler/brw_ir_vec4.h
> +++ b/src/intel/compiler/brw_ir_vec4.h
> @@ -43,6 +43,7 @@ public:
> src_reg(struct ::brw_reg reg);
>  
> bool equals(const src_reg ) const;
> +   bool negative_equals(const src_reg ) 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/intel/compiler/brw_reg.h b/src/intel/compiler/brw_reg.h
> index 7ad144b..732bddf 100644
> --- a/src/intel/compiler/brw_reg.h
> +++ b/src/intel/compiler/brw_reg.h
> @@ -255,6 +255,52 @@ brw_regs_equal(const struct brw_reg *a, const struct 
> brw_reg *b)
> return a->bits == b->bits && (df ? a->u64 == b->u64 : a->ud == b->ud);
>  }
>  
> +static inline bool
> +brw_regs_negative_equal(const struct brw_reg *a, const struct brw_reg *b)
> +{
> +   if (a->file == IMM) {
> +  if (a->bits != b->bits)
> + return false;
> +
> +  switch (a->type) {
> +  case BRW_REGISTER_TYPE_UQ:
> +  case BRW_REGISTER_TYPE_Q:
> + return a->d64 == -b->d64;
> +  case BRW_REGISTER_TYPE_DF:
> + return a->df == -b->df;
> +  case BRW_REGISTER_TYPE_UD:
> +  case BRW_REGISTER_TYPE_D:
> + return a->d == -b->d;
> +  case BRW_REGISTER_TYPE_F:
> + return a->f == -b->f;
> +  case BRW_REGISTER_TYPE_VF:
> + /* It is tempting to treat 0 as a negation of 0 (and -0 as a 
> negation
> +  * of -0).  There are occasions where 0 or -0 is used and the 
> exact
> +  * bit pattern is desired.  At the very least, changing this to 
> allow
> +  * 0 as a negation of 0 causes some fp64 tests to fail on IVB.
> +  */
> + return a->ud == (b->ud ^ 0x80808080);
> +  case BRW_REGISTER_TYPE_UW:
> +  case BRW_REGISTER_TYPE_W:
> +  case BRW_REGISTER_TYPE_UV:
> +  case BRW_REGISTER_TYPE_V:
> +  case BRW_REGISTER_TYPE_HF:
> +  case BRW_REGISTER_TYPE_UB:
> +  case BRW_REGISTER_TYPE_B:
> + /* FINISHME: Implement support for these types. */
 Is this missing functionality on purpose or the patch is still a wip? If
 it is the former, perhaps it would be good to explain why it is ok to
 leave that 

Re: [Mesa-dev] [PATCH 1/6] i965: Add negative_equals methods

[Ian Romanick]

On 03/23/2018 01:21 AM, Alejandro Piñeiro wrote:
> On 22/03/18 19:08, Ian Romanick wrote:
>> On 03/22/2018 01:12 AM, Alejandro Piñeiro wrote:
>>> Looks good in general, just a comment below.
>>>
>>>
>>> On 22/03/18 01:58, Ian Romanick wrote:
 From: Ian Romanick 

 This method is similar to the existing ::equals methods.  Instead of
 testing that two src_regs are equal to each other, it tests that one is
 the negation of the other.

 v2: Simplify various checks based on suggestions from Matt.  Use
 src_reg::type instead of fixed_hw_reg.type in a check.  Also suggested
 by Matt.

 v3: Rebase on 3 years.  Fix some problems with negative_equals with VF
 constants.  Add fs_reg::negative_equals.

 Signed-off-by: Ian Romanick 
 ---
  src/intel/compiler/brw_fs.cpp |  7 ++
  src/intel/compiler/brw_ir_fs.h|  1 +
  src/intel/compiler/brw_ir_vec4.h  |  1 +
  src/intel/compiler/brw_reg.h  | 46 
 +++
  src/intel/compiler/brw_shader.cpp |  6 +
  src/intel/compiler/brw_shader.h   |  1 +
  src/intel/compiler/brw_vec4.cpp   |  7 ++
  7 files changed, 69 insertions(+)

 diff --git a/src/intel/compiler/brw_fs.cpp b/src/intel/compiler/brw_fs.cpp
 index 6eea532..3d454c3 100644
 --- a/src/intel/compiler/brw_fs.cpp
 +++ b/src/intel/compiler/brw_fs.cpp
 @@ -454,6 +454,13 @@ fs_reg::equals(const fs_reg ) const
  }
  
  bool
 +fs_reg::negative_equals(const fs_reg ) const
 +{
 +   return (this->backend_reg::negative_equals(r) &&
 +   stride == r.stride);
 +}
 +
 +bool
  fs_reg::is_contiguous() const
  {
 return stride == 1;
 diff --git a/src/intel/compiler/brw_ir_fs.h 
 b/src/intel/compiler/brw_ir_fs.h
 index 54797ff..f06a33c 100644
 --- a/src/intel/compiler/brw_ir_fs.h
 +++ b/src/intel/compiler/brw_ir_fs.h
 @@ -41,6 +41,7 @@ public:
 fs_reg(enum brw_reg_file file, int nr, enum brw_reg_type type);
  
 bool equals(const fs_reg ) const;
 +   bool negative_equals(const fs_reg ) const;
 bool is_contiguous() const;
  
 /**
 diff --git a/src/intel/compiler/brw_ir_vec4.h 
 b/src/intel/compiler/brw_ir_vec4.h
 index cbaff2f..95c5119 100644
 --- a/src/intel/compiler/brw_ir_vec4.h
 +++ b/src/intel/compiler/brw_ir_vec4.h
 @@ -43,6 +43,7 @@ public:
 src_reg(struct ::brw_reg reg);
  
 bool equals(const src_reg ) const;
 +   bool negative_equals(const src_reg ) 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/intel/compiler/brw_reg.h b/src/intel/compiler/brw_reg.h
 index 7ad144b..732bddf 100644
 --- a/src/intel/compiler/brw_reg.h
 +++ b/src/intel/compiler/brw_reg.h
 @@ -255,6 +255,52 @@ brw_regs_equal(const struct brw_reg *a, const struct 
 brw_reg *b)
 return a->bits == b->bits && (df ? a->u64 == b->u64 : a->ud == b->ud);
  }
  
 +static inline bool
 +brw_regs_negative_equal(const struct brw_reg *a, const struct brw_reg *b)
 +{
 +   if (a->file == IMM) {
 +  if (a->bits != b->bits)
 + return false;
 +
 +  switch (a->type) {
 +  case BRW_REGISTER_TYPE_UQ:
 +  case BRW_REGISTER_TYPE_Q:
 + return a->d64 == -b->d64;
 +  case BRW_REGISTER_TYPE_DF:
 + return a->df == -b->df;
 +  case BRW_REGISTER_TYPE_UD:
 +  case BRW_REGISTER_TYPE_D:
 + return a->d == -b->d;
 +  case BRW_REGISTER_TYPE_F:
 + return a->f == -b->f;
 +  case BRW_REGISTER_TYPE_VF:
 + /* It is tempting to treat 0 as a negation of 0 (and -0 as a 
 negation
 +  * of -0).  There are occasions where 0 or -0 is used and the 
 exact
 +  * bit pattern is desired.  At the very least, changing this to 
 allow
 +  * 0 as a negation of 0 causes some fp64 tests to fail on IVB.
 +  */
 + return a->ud == (b->ud ^ 0x80808080);
 +  case BRW_REGISTER_TYPE_UW:
 +  case BRW_REGISTER_TYPE_W:
 +  case BRW_REGISTER_TYPE_UV:
 +  case BRW_REGISTER_TYPE_V:
 +  case BRW_REGISTER_TYPE_HF:
 +  case BRW_REGISTER_TYPE_UB:
 +  case BRW_REGISTER_TYPE_B:
 + /* FINISHME: Implement support for these types. */
>>> Is this missing functionality on purpose or the patch is still a wip? If
>>> it is the former, perhaps it would be good to explain why it is ok to
>>> leave that functionality hole.
>> Basically this means that any optimizations that depend on
>> negative_equals won't happen.  I didn't implement these paths because,
>> as far as I can tell, 

Re: [Mesa-dev] [PATCH 1/6] i965: Add negative_equals methods

[Alejandro Piñeiro]

On 22/03/18 19:08, Ian Romanick wrote:
> On 03/22/2018 01:12 AM, Alejandro Piñeiro wrote:
>> Looks good in general, just a comment below.
>>
>>
>> On 22/03/18 01:58, Ian Romanick wrote:
>>> From: Ian Romanick 
>>>
>>> This method is similar to the existing ::equals methods.  Instead of
>>> testing that two src_regs are equal to each other, it tests that one is
>>> the negation of the other.
>>>
>>> v2: Simplify various checks based on suggestions from Matt.  Use
>>> src_reg::type instead of fixed_hw_reg.type in a check.  Also suggested
>>> by Matt.
>>>
>>> v3: Rebase on 3 years.  Fix some problems with negative_equals with VF
>>> constants.  Add fs_reg::negative_equals.
>>>
>>> Signed-off-by: Ian Romanick 
>>> ---
>>>  src/intel/compiler/brw_fs.cpp |  7 ++
>>>  src/intel/compiler/brw_ir_fs.h|  1 +
>>>  src/intel/compiler/brw_ir_vec4.h  |  1 +
>>>  src/intel/compiler/brw_reg.h  | 46 
>>> +++
>>>  src/intel/compiler/brw_shader.cpp |  6 +
>>>  src/intel/compiler/brw_shader.h   |  1 +
>>>  src/intel/compiler/brw_vec4.cpp   |  7 ++
>>>  7 files changed, 69 insertions(+)
>>>
>>> diff --git a/src/intel/compiler/brw_fs.cpp b/src/intel/compiler/brw_fs.cpp
>>> index 6eea532..3d454c3 100644
>>> --- a/src/intel/compiler/brw_fs.cpp
>>> +++ b/src/intel/compiler/brw_fs.cpp
>>> @@ -454,6 +454,13 @@ fs_reg::equals(const fs_reg ) const
>>>  }
>>>  
>>>  bool
>>> +fs_reg::negative_equals(const fs_reg ) const
>>> +{
>>> +   return (this->backend_reg::negative_equals(r) &&
>>> +   stride == r.stride);
>>> +}
>>> +
>>> +bool
>>>  fs_reg::is_contiguous() const
>>>  {
>>> return stride == 1;
>>> diff --git a/src/intel/compiler/brw_ir_fs.h b/src/intel/compiler/brw_ir_fs.h
>>> index 54797ff..f06a33c 100644
>>> --- a/src/intel/compiler/brw_ir_fs.h
>>> +++ b/src/intel/compiler/brw_ir_fs.h
>>> @@ -41,6 +41,7 @@ public:
>>> fs_reg(enum brw_reg_file file, int nr, enum brw_reg_type type);
>>>  
>>> bool equals(const fs_reg ) const;
>>> +   bool negative_equals(const fs_reg ) const;
>>> bool is_contiguous() const;
>>>  
>>> /**
>>> diff --git a/src/intel/compiler/brw_ir_vec4.h 
>>> b/src/intel/compiler/brw_ir_vec4.h
>>> index cbaff2f..95c5119 100644
>>> --- a/src/intel/compiler/brw_ir_vec4.h
>>> +++ b/src/intel/compiler/brw_ir_vec4.h
>>> @@ -43,6 +43,7 @@ public:
>>> src_reg(struct ::brw_reg reg);
>>>  
>>> bool equals(const src_reg ) const;
>>> +   bool negative_equals(const src_reg ) 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/intel/compiler/brw_reg.h b/src/intel/compiler/brw_reg.h
>>> index 7ad144b..732bddf 100644
>>> --- a/src/intel/compiler/brw_reg.h
>>> +++ b/src/intel/compiler/brw_reg.h
>>> @@ -255,6 +255,52 @@ brw_regs_equal(const struct brw_reg *a, const struct 
>>> brw_reg *b)
>>> return a->bits == b->bits && (df ? a->u64 == b->u64 : a->ud == b->ud);
>>>  }
>>>  
>>> +static inline bool
>>> +brw_regs_negative_equal(const struct brw_reg *a, const struct brw_reg *b)
>>> +{
>>> +   if (a->file == IMM) {
>>> +  if (a->bits != b->bits)
>>> + return false;
>>> +
>>> +  switch (a->type) {
>>> +  case BRW_REGISTER_TYPE_UQ:
>>> +  case BRW_REGISTER_TYPE_Q:
>>> + return a->d64 == -b->d64;
>>> +  case BRW_REGISTER_TYPE_DF:
>>> + return a->df == -b->df;
>>> +  case BRW_REGISTER_TYPE_UD:
>>> +  case BRW_REGISTER_TYPE_D:
>>> + return a->d == -b->d;
>>> +  case BRW_REGISTER_TYPE_F:
>>> + return a->f == -b->f;
>>> +  case BRW_REGISTER_TYPE_VF:
>>> + /* It is tempting to treat 0 as a negation of 0 (and -0 as a 
>>> negation
>>> +  * of -0).  There are occasions where 0 or -0 is used and the 
>>> exact
>>> +  * bit pattern is desired.  At the very least, changing this to 
>>> allow
>>> +  * 0 as a negation of 0 causes some fp64 tests to fail on IVB.
>>> +  */
>>> + return a->ud == (b->ud ^ 0x80808080);
>>> +  case BRW_REGISTER_TYPE_UW:
>>> +  case BRW_REGISTER_TYPE_W:
>>> +  case BRW_REGISTER_TYPE_UV:
>>> +  case BRW_REGISTER_TYPE_V:
>>> +  case BRW_REGISTER_TYPE_HF:
>>> +  case BRW_REGISTER_TYPE_UB:
>>> +  case BRW_REGISTER_TYPE_B:
>>> + /* FINISHME: Implement support for these types. */
>> Is this missing functionality on purpose or the patch is still a wip? If
>> it is the former, perhaps it would be good to explain why it is ok to
>> leave that functionality hole.
> Basically this means that any optimizations that depend on
> negative_equals won't happen.  I didn't implement these paths because,
> as far as I can tell, we never generate any code that uses these types.
> I was a bit reluctant to implement something that I couldn't test.

Hmm, all that reasoning seems to go for a 

Re: [Mesa-dev] [PATCH 1/6] i965: Add negative_equals methods

[Ian Romanick]

On 03/22/2018 01:12 AM, Alejandro Piñeiro wrote:
> Looks good in general, just a comment below.
> 
> 
> On 22/03/18 01:58, Ian Romanick wrote:
>> From: Ian Romanick 
>>
>> This method is similar to the existing ::equals methods.  Instead of
>> testing that two src_regs are equal to each other, it tests that one is
>> the negation of the other.
>>
>> v2: Simplify various checks based on suggestions from Matt.  Use
>> src_reg::type instead of fixed_hw_reg.type in a check.  Also suggested
>> by Matt.
>>
>> v3: Rebase on 3 years.  Fix some problems with negative_equals with VF
>> constants.  Add fs_reg::negative_equals.
>>
>> Signed-off-by: Ian Romanick 
>> ---
>>  src/intel/compiler/brw_fs.cpp |  7 ++
>>  src/intel/compiler/brw_ir_fs.h|  1 +
>>  src/intel/compiler/brw_ir_vec4.h  |  1 +
>>  src/intel/compiler/brw_reg.h  | 46 
>> +++
>>  src/intel/compiler/brw_shader.cpp |  6 +
>>  src/intel/compiler/brw_shader.h   |  1 +
>>  src/intel/compiler/brw_vec4.cpp   |  7 ++
>>  7 files changed, 69 insertions(+)
>>
>> diff --git a/src/intel/compiler/brw_fs.cpp b/src/intel/compiler/brw_fs.cpp
>> index 6eea532..3d454c3 100644
>> --- a/src/intel/compiler/brw_fs.cpp
>> +++ b/src/intel/compiler/brw_fs.cpp
>> @@ -454,6 +454,13 @@ fs_reg::equals(const fs_reg ) const
>>  }
>>  
>>  bool
>> +fs_reg::negative_equals(const fs_reg ) const
>> +{
>> +   return (this->backend_reg::negative_equals(r) &&
>> +   stride == r.stride);
>> +}
>> +
>> +bool
>>  fs_reg::is_contiguous() const
>>  {
>> return stride == 1;
>> diff --git a/src/intel/compiler/brw_ir_fs.h b/src/intel/compiler/brw_ir_fs.h
>> index 54797ff..f06a33c 100644
>> --- a/src/intel/compiler/brw_ir_fs.h
>> +++ b/src/intel/compiler/brw_ir_fs.h
>> @@ -41,6 +41,7 @@ public:
>> fs_reg(enum brw_reg_file file, int nr, enum brw_reg_type type);
>>  
>> bool equals(const fs_reg ) const;
>> +   bool negative_equals(const fs_reg ) const;
>> bool is_contiguous() const;
>>  
>> /**
>> diff --git a/src/intel/compiler/brw_ir_vec4.h 
>> b/src/intel/compiler/brw_ir_vec4.h
>> index cbaff2f..95c5119 100644
>> --- a/src/intel/compiler/brw_ir_vec4.h
>> +++ b/src/intel/compiler/brw_ir_vec4.h
>> @@ -43,6 +43,7 @@ public:
>> src_reg(struct ::brw_reg reg);
>>  
>> bool equals(const src_reg ) const;
>> +   bool negative_equals(const src_reg ) 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/intel/compiler/brw_reg.h b/src/intel/compiler/brw_reg.h
>> index 7ad144b..732bddf 100644
>> --- a/src/intel/compiler/brw_reg.h
>> +++ b/src/intel/compiler/brw_reg.h
>> @@ -255,6 +255,52 @@ brw_regs_equal(const struct brw_reg *a, const struct 
>> brw_reg *b)
>> return a->bits == b->bits && (df ? a->u64 == b->u64 : a->ud == b->ud);
>>  }
>>  
>> +static inline bool
>> +brw_regs_negative_equal(const struct brw_reg *a, const struct brw_reg *b)
>> +{
>> +   if (a->file == IMM) {
>> +  if (a->bits != b->bits)
>> + return false;
>> +
>> +  switch (a->type) {
>> +  case BRW_REGISTER_TYPE_UQ:
>> +  case BRW_REGISTER_TYPE_Q:
>> + return a->d64 == -b->d64;
>> +  case BRW_REGISTER_TYPE_DF:
>> + return a->df == -b->df;
>> +  case BRW_REGISTER_TYPE_UD:
>> +  case BRW_REGISTER_TYPE_D:
>> + return a->d == -b->d;
>> +  case BRW_REGISTER_TYPE_F:
>> + return a->f == -b->f;
>> +  case BRW_REGISTER_TYPE_VF:
>> + /* It is tempting to treat 0 as a negation of 0 (and -0 as a 
>> negation
>> +  * of -0).  There are occasions where 0 or -0 is used and the exact
>> +  * bit pattern is desired.  At the very least, changing this to 
>> allow
>> +  * 0 as a negation of 0 causes some fp64 tests to fail on IVB.
>> +  */
>> + return a->ud == (b->ud ^ 0x80808080);
>> +  case BRW_REGISTER_TYPE_UW:
>> +  case BRW_REGISTER_TYPE_W:
>> +  case BRW_REGISTER_TYPE_UV:
>> +  case BRW_REGISTER_TYPE_V:
>> +  case BRW_REGISTER_TYPE_HF:
>> +  case BRW_REGISTER_TYPE_UB:
>> +  case BRW_REGISTER_TYPE_B:
>> + /* FINISHME: Implement support for these types. */
> 
> Is this missing functionality on purpose or the patch is still a wip? If
> it is the former, perhaps it would be good to explain why it is ok to
> leave that functionality hole.

Basically this means that any optimizations that depend on
negative_equals won't happen.  I didn't implement these paths because,
as far as I can tell, we never generate any code that uses these types.
I was a bit reluctant to implement something that I couldn't test.

>> + return false;
>> +  default:
>> + unreachable("not reached");
>> +  }
>> +   } else {
>> +  struct brw_reg tmp = *a;
>> +
>> +  tmp.negate = !tmp.negate;
>> +
>> +  return brw_regs_equal(, 

Re: [Mesa-dev] [PATCH 1/6] i965: Add negative_equals methods

[Alejandro Piñeiro]

Looks good in general, just a comment below.


On 22/03/18 01:58, Ian Romanick wrote:
> From: Ian Romanick 
>
> This method is similar to the existing ::equals methods.  Instead of
> testing that two src_regs are equal to each other, it tests that one is
> the negation of the other.
>
> v2: Simplify various checks based on suggestions from Matt.  Use
> src_reg::type instead of fixed_hw_reg.type in a check.  Also suggested
> by Matt.
>
> v3: Rebase on 3 years.  Fix some problems with negative_equals with VF
> constants.  Add fs_reg::negative_equals.
>
> Signed-off-by: Ian Romanick 
> ---
>  src/intel/compiler/brw_fs.cpp |  7 ++
>  src/intel/compiler/brw_ir_fs.h|  1 +
>  src/intel/compiler/brw_ir_vec4.h  |  1 +
>  src/intel/compiler/brw_reg.h  | 46 
> +++
>  src/intel/compiler/brw_shader.cpp |  6 +
>  src/intel/compiler/brw_shader.h   |  1 +
>  src/intel/compiler/brw_vec4.cpp   |  7 ++
>  7 files changed, 69 insertions(+)
>
> diff --git a/src/intel/compiler/brw_fs.cpp b/src/intel/compiler/brw_fs.cpp
> index 6eea532..3d454c3 100644
> --- a/src/intel/compiler/brw_fs.cpp
> +++ b/src/intel/compiler/brw_fs.cpp
> @@ -454,6 +454,13 @@ fs_reg::equals(const fs_reg ) const
>  }
>  
>  bool
> +fs_reg::negative_equals(const fs_reg ) const
> +{
> +   return (this->backend_reg::negative_equals(r) &&
> +   stride == r.stride);
> +}
> +
> +bool
>  fs_reg::is_contiguous() const
>  {
> return stride == 1;
> diff --git a/src/intel/compiler/brw_ir_fs.h b/src/intel/compiler/brw_ir_fs.h
> index 54797ff..f06a33c 100644
> --- a/src/intel/compiler/brw_ir_fs.h
> +++ b/src/intel/compiler/brw_ir_fs.h
> @@ -41,6 +41,7 @@ public:
> fs_reg(enum brw_reg_file file, int nr, enum brw_reg_type type);
>  
> bool equals(const fs_reg ) const;
> +   bool negative_equals(const fs_reg ) const;
> bool is_contiguous() const;
>  
> /**
> diff --git a/src/intel/compiler/brw_ir_vec4.h 
> b/src/intel/compiler/brw_ir_vec4.h
> index cbaff2f..95c5119 100644
> --- a/src/intel/compiler/brw_ir_vec4.h
> +++ b/src/intel/compiler/brw_ir_vec4.h
> @@ -43,6 +43,7 @@ public:
> src_reg(struct ::brw_reg reg);
>  
> bool equals(const src_reg ) const;
> +   bool negative_equals(const src_reg ) 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/intel/compiler/brw_reg.h b/src/intel/compiler/brw_reg.h
> index 7ad144b..732bddf 100644
> --- a/src/intel/compiler/brw_reg.h
> +++ b/src/intel/compiler/brw_reg.h
> @@ -255,6 +255,52 @@ brw_regs_equal(const struct brw_reg *a, const struct 
> brw_reg *b)
> return a->bits == b->bits && (df ? a->u64 == b->u64 : a->ud == b->ud);
>  }
>  
> +static inline bool
> +brw_regs_negative_equal(const struct brw_reg *a, const struct brw_reg *b)
> +{
> +   if (a->file == IMM) {
> +  if (a->bits != b->bits)
> + return false;
> +
> +  switch (a->type) {
> +  case BRW_REGISTER_TYPE_UQ:
> +  case BRW_REGISTER_TYPE_Q:
> + return a->d64 == -b->d64;
> +  case BRW_REGISTER_TYPE_DF:
> + return a->df == -b->df;
> +  case BRW_REGISTER_TYPE_UD:
> +  case BRW_REGISTER_TYPE_D:
> + return a->d == -b->d;
> +  case BRW_REGISTER_TYPE_F:
> + return a->f == -b->f;
> +  case BRW_REGISTER_TYPE_VF:
> + /* It is tempting to treat 0 as a negation of 0 (and -0 as a 
> negation
> +  * of -0).  There are occasions where 0 or -0 is used and the exact
> +  * bit pattern is desired.  At the very least, changing this to 
> allow
> +  * 0 as a negation of 0 causes some fp64 tests to fail on IVB.
> +  */
> + return a->ud == (b->ud ^ 0x80808080);
> +  case BRW_REGISTER_TYPE_UW:
> +  case BRW_REGISTER_TYPE_W:
> +  case BRW_REGISTER_TYPE_UV:
> +  case BRW_REGISTER_TYPE_V:
> +  case BRW_REGISTER_TYPE_HF:
> +  case BRW_REGISTER_TYPE_UB:
> +  case BRW_REGISTER_TYPE_B:
> + /* FINISHME: Implement support for these types. */

Is this missing functionality on purpose or the patch is still a wip? If
it is the former, perhaps it would be good to explain why it is ok to
leave that functionality hole.

> + return false;
> +  default:
> + unreachable("not reached");
> +  }
> +   } else {
> +  struct brw_reg tmp = *a;
> +
> +  tmp.negate = !tmp.negate;
> +
> +  return brw_regs_equal(, b);
> +   }
> +}
> +
>  struct brw_indirect {
> unsigned addr_subnr:4;
> int addr_offset:10;
> diff --git a/src/intel/compiler/brw_shader.cpp 
> b/src/intel/compiler/brw_shader.cpp
> index 054962b..9cdf9fc 100644
> --- a/src/intel/compiler/brw_shader.cpp
> +++ b/src/intel/compiler/brw_shader.cpp
> @@ -685,6 +685,12 @@ backend_reg::equals(const backend_reg ) const
>  }
>  
>  bool
> +backend_reg::negative_equals(const backend_reg ) const
> +{
> 

[Mesa-dev] [PATCH 1/6] i965: Add negative_equals methods

[Ian Romanick]

From: Ian Romanick 

This method is similar to the existing ::equals methods.  Instead of
testing that two src_regs are equal to each other, it tests that one is
the negation of the other.

v2: Simplify various checks based on suggestions from Matt.  Use
src_reg::type instead of fixed_hw_reg.type in a check.  Also suggested
by Matt.

v3: Rebase on 3 years.  Fix some problems with negative_equals with VF
constants.  Add fs_reg::negative_equals.

Signed-off-by: Ian Romanick 
---
 src/intel/compiler/brw_fs.cpp |  7 ++
 src/intel/compiler/brw_ir_fs.h|  1 +
 src/intel/compiler/brw_ir_vec4.h  |  1 +
 src/intel/compiler/brw_reg.h  | 46 +++
 src/intel/compiler/brw_shader.cpp |  6 +
 src/intel/compiler/brw_shader.h   |  1 +
 src/intel/compiler/brw_vec4.cpp   |  7 ++
 7 files changed, 69 insertions(+)

diff --git a/src/intel/compiler/brw_fs.cpp b/src/intel/compiler/brw_fs.cpp
index 6eea532..3d454c3 100644
--- a/src/intel/compiler/brw_fs.cpp
+++ b/src/intel/compiler/brw_fs.cpp
@@ -454,6 +454,13 @@ fs_reg::equals(const fs_reg ) const
 }
 
 bool
+fs_reg::negative_equals(const fs_reg ) const
+{
+   return (this->backend_reg::negative_equals(r) &&
+   stride == r.stride);
+}
+
+bool
 fs_reg::is_contiguous() const
 {
return stride == 1;
diff --git a/src/intel/compiler/brw_ir_fs.h b/src/intel/compiler/brw_ir_fs.h
index 54797ff..f06a33c 100644
--- a/src/intel/compiler/brw_ir_fs.h
+++ b/src/intel/compiler/brw_ir_fs.h
@@ -41,6 +41,7 @@ public:
fs_reg(enum brw_reg_file file, int nr, enum brw_reg_type type);
 
bool equals(const fs_reg ) const;
+   bool negative_equals(const fs_reg ) const;
bool is_contiguous() const;
 
/**
diff --git a/src/intel/compiler/brw_ir_vec4.h b/src/intel/compiler/brw_ir_vec4.h
index cbaff2f..95c5119 100644
--- a/src/intel/compiler/brw_ir_vec4.h
+++ b/src/intel/compiler/brw_ir_vec4.h
@@ -43,6 +43,7 @@ public:
src_reg(struct ::brw_reg reg);
 
bool equals(const src_reg ) const;
+   bool negative_equals(const src_reg ) 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/intel/compiler/brw_reg.h b/src/intel/compiler/brw_reg.h
index 7ad144b..732bddf 100644
--- a/src/intel/compiler/brw_reg.h
+++ b/src/intel/compiler/brw_reg.h
@@ -255,6 +255,52 @@ brw_regs_equal(const struct brw_reg *a, const struct 
brw_reg *b)
return a->bits == b->bits && (df ? a->u64 == b->u64 : a->ud == b->ud);
 }
 
+static inline bool
+brw_regs_negative_equal(const struct brw_reg *a, const struct brw_reg *b)
+{
+   if (a->file == IMM) {
+  if (a->bits != b->bits)
+ return false;
+
+  switch (a->type) {
+  case BRW_REGISTER_TYPE_UQ:
+  case BRW_REGISTER_TYPE_Q:
+ return a->d64 == -b->d64;
+  case BRW_REGISTER_TYPE_DF:
+ return a->df == -b->df;
+  case BRW_REGISTER_TYPE_UD:
+  case BRW_REGISTER_TYPE_D:
+ return a->d == -b->d;
+  case BRW_REGISTER_TYPE_F:
+ return a->f == -b->f;
+  case BRW_REGISTER_TYPE_VF:
+ /* It is tempting to treat 0 as a negation of 0 (and -0 as a negation
+  * of -0).  There are occasions where 0 or -0 is used and the exact
+  * bit pattern is desired.  At the very least, changing this to allow
+  * 0 as a negation of 0 causes some fp64 tests to fail on IVB.
+  */
+ return a->ud == (b->ud ^ 0x80808080);
+  case BRW_REGISTER_TYPE_UW:
+  case BRW_REGISTER_TYPE_W:
+  case BRW_REGISTER_TYPE_UV:
+  case BRW_REGISTER_TYPE_V:
+  case BRW_REGISTER_TYPE_HF:
+  case BRW_REGISTER_TYPE_UB:
+  case BRW_REGISTER_TYPE_B:
+ /* FINISHME: Implement support for these types. */
+ return false;
+  default:
+ unreachable("not reached");
+  }
+   } else {
+  struct brw_reg tmp = *a;
+
+  tmp.negate = !tmp.negate;
+
+  return brw_regs_equal(, b);
+   }
+}
+
 struct brw_indirect {
unsigned addr_subnr:4;
int addr_offset:10;
diff --git a/src/intel/compiler/brw_shader.cpp 
b/src/intel/compiler/brw_shader.cpp
index 054962b..9cdf9fc 100644
--- a/src/intel/compiler/brw_shader.cpp
+++ b/src/intel/compiler/brw_shader.cpp
@@ -685,6 +685,12 @@ backend_reg::equals(const backend_reg ) const
 }
 
 bool
+backend_reg::negative_equals(const backend_reg ) const
+{
+   return brw_regs_negative_equal(this, ) && offset == r.offset;
+}
+
+bool
 backend_reg::is_zero() const
 {
if (file != IMM)
diff --git a/src/intel/compiler/brw_shader.h b/src/intel/compiler/brw_shader.h
index fd02feb..7d97ddb 100644
--- a/src/intel/compiler/brw_shader.h
+++ b/src/intel/compiler/brw_shader.h
@@ -59,6 +59,7 @@ struct backend_reg : private brw_reg
}
 
bool equals(const backend_reg ) const;
+   bool negative_equals(const backend_reg ) const;
 
bool is_zero() const;
bool is_one() const;
diff --git