[Mesa-dev] [PATCH 4/5] i965/vs: Allow CSE to handle MULs with negated arguments.

[Matt Turner]

On Thu, Apr 9, 2015 at 10:44 AM, Ian Romanick <idr at freedesktop.org> wrote:
> On 04/09/2015 09:37 AM, Matt Turner wrote:
>> On Wed, Apr 8, 2015 at 4:38 PM, Ian Romanick <idr at freedesktop.org> wrote:
>>> From: Ian Romanick <ian.d.romanick at intel.com>
>>>
>>> This is similar to commit (47c4b38: i965/fs: Allow CSE to handle MULs
>>> with negated arguments.), but it uses a slightly different approach.
>>
>> Just repeating myself... I don't know why this is different if it
>> doesn't do something more. Anyway,
>
> Readability and extensibility.  Keeping the "are these negated
> operations" and "do negated operands produce the same result" logic
> separate, it's easier to see what's going on, and it's easier to add to
> it later.

I mean, they're pretty interlinked. Operations like mul, div, and dot
products operate one way, and addition operates another. Multiply-add
is yet another. I don't think you can really get away from that.

Presumably you found the code I added to brw_fs_cse.cpp distasteful
(is that why no one reviewed it in two attempts over seven weeks?!),
but it's 28 lines total, with 4 calls to equals() (the same as other
commutative operations). This code has 8 calls to equals() and another
8 to negative_equals(), which itself is 38 lines.

> It also matched the way I did the (currently NAK'ed) changes at the GLSL
> level, so my brain was already in that space.
>
>> Reviewed-by: Matt Turner <mattst88 at gmail.com>
>