[Mesa-dev] [PATCH 0/2] Nir: Allow CSE of SSBO loads

[Iago Toral]

On Thu, 2015-10-22 at 09:09 -0700, Jason Ekstrand wrote:
> On Thu, Oct 22, 2015 at 4:21 AM, Iago Toral Quiroga <itoral at igalia.com> wrote:
> > I implemented this first as a separate optimization pass in GLSL IR [1], but
> > Curro pointed out that this being pretty much a restricted form of a CSE pass
> > it would probably make more sense to do it inside CSE (and we no longer have
> > a CSE pass in GLSL IR).
> >
> > Unlike other things we CSE in NIR, in the case of SSBO loads we need to make
> > sure that we invalidate previous entries in the set in the presence of
> > conflicting instructions (i.e. SSBO writes to the same block and offset) or
> > in the presence of memory barriers.
> >
> > If this is accepted I intend to extend this to also cover image reads, which
> > follow similar behavior.
> >
> > No regressions observed in piglit or dEQP's SSBO functional tests.
> >
> > [1] http://lists.freedesktop.org/archives/mesa-dev/2015-October/097718.html
> 
> I think you've gotten enough NAK's that I don't need to chime in
> there.  Unfortunately, solving this in general is something of a
> research project that both Connor and I have been thinking about for
> quite some time.  I've been thinking off-and-on about how to add a
> proper memory model to lower_vars_to_ssa for almost a year now and
> still haven't come up with a good way to do it.  I don't know whether
> SSBO's would be simpler or not.  We need a proper memory model for
> both lower_vars_to_ssa and SSBO load/stores (and shared local
> variables) but it's a substantial research project.
> 
> This isn't to say that you couldn't do it.  Just know what you're taking on. ;-)

Yeah, it does not make sense that I try to do this, you guys have
clearly given this much more thought than me and know much better how a
solution for this would fit in NIR than me.

> That said, here's a suggestion for something that we *could* write
> today, wouldn't be very hard, and wold solve a decent number of cases.
> 
> For each block:
> 
> 1) Create a new instruction set (don't use anything from any previous blocks)
> 2) call add_or_rewrite on all ssbo load operations
> 3) If you ever see a barrier or ssbo store, destroy the entire
> instruction set and start again.

Yep, this is what I was thinking for the load-combine pass that Connor
suggested. However, I think that in this case we do not need to destroy
the entire set when we find a store, only for memory barriers, right? I
mean, there should be nothing preventing us from checking the
offset/block of the store and compare it with the offset/block of the
loads in the set to decide which ones we need to remove (like I was
doing in my last patch)

> This is something you could put together fairly quickly and would
> handle a fair number of cases.  With a little special casing, you may
> also be able to handle store and then an immediate load of the same
> value or duplicate stores.  Anything much more complex than that is
> going to take a lot more thought.

Yes, I'll give this a try next. Thanks for all the comments and
suggestions!

Iago