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

[Paul Berry]

On 4 October 2013 14:24, Matt Turner <mattst88 at gmail.com> wrote:

> 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.
>

Sadly I don't think we can rely on that.  Since we haven't (yet)
implemented SSA, there's no guarantee that different writes will use
different virtual GRFs.  They will only be different virtual GRFs if they
are different variables in the GLSL IR.  And the GLSL IR optimization
passes (e.g. opt_copy_propagation and opt_tree_grafting) are pretty
aggressive in trying to get rid of extra GLSL IR variables where possible,
so even if they start off as different variables in the GLSL IR there's no
guarantee that they'll still be different variables by the time we hit the
back end.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20131004/52713119/attachment.html>