[Mesa-dev] [PATCH 09/10] i965/fs: Add a peephole pass to combine ADD with ADDC/SUBB.

[Matt Turner]

On Fri, Oct 4, 2013 at 2:19 PM, Paul Berry <stereotype441 at gmail.com> wrote:
> On 4 October 2013 13:51, Paul Berry <stereotype441 at gmail.com> wrote:
>>
>> On 3 October 2013 10:59, Matt Turner <mattst88 at gmail.com> wrote:
>>>
>>> v2: Check fixed_hw_reg.{file,nr} instead of dst.reg.
>>> v3: Store the bool emitted_addc_or_subb in the class, not static.
>>> ---
>>>  src/mesa/drivers/dri/i965/brw_fs.h           |   3 +
>>>  src/mesa/drivers/dri/i965/brw_fs_visitor.cpp | 104
>>> +++++++++++++++++++++++++++
>>>  2 files changed, 107 insertions(+)
>>
>>
>> Reviewed-by: Paul Berry <stereotype441 at gmail.com>
>
>
> Whoops, wait a minute.  I spoke too soon.
>
> I'm concerned because it looks like this peephole pass is too generous.  The
> following instruction stream, for example, shoud not be peephole optimized:
>
> addc null, x, y
> mov carry, acc0
> mov x, z
> add sum x, y
>
> But I don't see anything in fs_visitor::try_combine_add_with_addc_subb() to
> prevent this.

We're operating on virtual grfs at this point, so I don't think that
case is possible. I.e., if there were an intervening write to x, it
just would have been a different vgrf. I think Ken had this same
concern at one point, although I don't think he ever said it via
email.