On Thu, Oct 22, 2015 at 11:13 PM, Iago Toral <ito...@igalia.com> wrote: > On Thu, 2015-10-22 at 09:09 -0700, Jason Ekstrand wrote: >> On Thu, Oct 22, 2015 at 4:21 AM, Iago Toral Quiroga <ito...@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)
That's where you get into the "special casing" I mentioned below. If you have an direct store, you would have to throw away any indirect loads and then insert a fake direct load for the given offset. If you have an indirect store, you would have to throw away everything and then insert a fake indirect load for the given offset. So, yes, you can do it, but it'll take a little more work. You'll also probably want to use a different hashing function than we use for CSE because you need to be able to handle both real and fake loads and have them compare/hash the same. Does that make sense? --Jason >> 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 > _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev