[Mesa-dev] [PATCH 00/11] ARB_shader_atomic_counter_ops for NIR and i965

[Iago Toral]

On Wed, 2016-07-06 at 09:36 +0200, Iago Toral wrote:
> On Tue, 2016-07-05 at 17:46 -0700, Ian Romanick wrote:
> > 
> > The first 7 patches in this series put GLSL-to-NIR on a small
> > diet.  I
> > looked at the giant sequense of 'if (strcmp(...) == 0) { ... } else
> > if
> > (strcmp(...) == 0) { ...' and said, "Oh hell no."  I don't think we
> > care
> > much about the performance of this code, so I opted to tune for
> > size.
> > Using an in-code radix trie gets it about as small as I think it
> > can
> > get.  The result is -784 bytes in a single function.  All 41
> > strings
> > just disappear.
> Yeah, this looks like a nice clean-up. Patches 1-4 are:
> Reviewed-by: Iago Toral Quiroga <itoral at igalia.com>

I dropped a few minor comments for patch 9, but otherwise patches 8-11
are:

Reviewed-by: Iago Toral Quiroga <itoral at igalia.com>

Patches 5-7 should probably be reviewed by someone more experienced
with Python/mako, although I am not sure if the extra complexity added
with those patches pays off :-/. In any case, I would be happy to
review those too if you can't find someone more appropriate, just let
me know if that is the case.

> > 
> > It looks like src/mesa/state_tracker/st_glsl_to_tgsi.cpp could get
> > similar treatment, and the savings there should be even larger.  My
> > recommendation would be to copy
> > src/compiler/glsl/nir_intrinsic_map.py
> > into src/mesa/state_tracker and change it to suit the needs of that
> > code.  The hard part is already done. :)
> > 
> > The rest of the series adds the new intrinsics to NIR and to the
> > i965
> > driver.
> > 
> > What we don't have is a good set of piglit tests for the new
> > intrinsics.
> > We also might not have tests for the existing flavors of the new
> > intrinsics on, for example, SSBOs.  There is a test for
> > atomicCounterAddARB.  I think it's going to be fairly difficult to
> > come
> > up with good tests for the other functions.  I'll have to think
> > about
> > it
> > some more.
> > 
> > _______________________________________________
> > mesa-dev mailing list
> > mesa-dev at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/mesa-dev
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev