[Mesa-dev] [PATCH v3 00/14] add support for ARB_compute_variable_group_size

[Nicolai Hähnle]

On 26.09.2016 19:23, Samuel Pitoiset wrote:
> v3: - use a new case statement in r600_pipe_common.c
>     - fix compilation with softpipe
>     - initialize max_variable_threads_per_block to 0

I have sent some remarks on patches 2 and 4. Patches 1, 3, 5-11:

Reviewed-by: Nicolai Hähnle <nicolai.haehnle at amd.com>

>
> v2: - update formatting spec quotations
>     - add PIPE_COMPUTE_CAP_MAX_VARIABLE_THREADS_PER_BLOCK
>     - expose the ext based on that new cap
>     - add missing relnotes
>     - various cosmetic changes
>
> From original cover-letter:
>
> Hi,
>
> This series implements ARB_compute_variable_group_size written against GL 4.3.
> This extension allows to dispatch variable work group size via a new function
> called glDispatchComputeGroupSizeARB().
>
> Because this extension is pretty similar to ARB_compute_shader, all Gallium
> drivers which already support compute shaders will expose
> ARB_compute_variable_group_size with that series.
>
> I did write a bunch of piglit tests, have a look here if you want:
> https://lists.freedesktop.org/archives/piglit/2016-September/020755.html
>
> All tests pass on Fermi (GF119) as well as all previous compute shaders tests.
>
> Marek, Nicolai and other AMD folks, I don't know if radeonsi will need a fix
> somewhere for handling a variable work group size, but as I don't have the
> hardware, I can't test. Let me know if something needs to be slighty updated.
>
> Please review,
> Thanks!
>
> Samuel Pitoiset (14):
>   glapi: add entry points for GL_ARB_compute_variable_group_size
>   mesa/main: add support for ARB_compute_variable_groups_size
>   glsl: add enable flags for ARB_compute_variable_group_size
>   glsl: process local_size_variable input qualifier
>   glsl: reject compute shaders with fixed and variable local size
>   glsl/linker: handle errors when a variable local size is used
>   glsl: add gl_LocalGroupSizeARB as a system value
>   gallium: add PIPE_COMPUTE_CAP_MAX_VARIABLE_THREADS_PER_BLOCK
>   st/mesa: add mapping for SYSTEM_VALUE_LOCAL_GROUP_SIZE
>   st/mesa: add support for dispatching a variable local size
>   st/mesa: expose ARB_compute_variable_group_size
>   nv50/ir: use 1024 threads/block for variable local size
>   nvc0: expose ARB_compute_variable_group_size
>   docs: mark ARB_compute_variable_group_size as done for nvc0
>
>  docs/features.txt                                  |  2 +-
>  docs/relnotes/12.1.0.html                          |  1 +
>  src/compiler/glsl/ast.h                            |  5 ++
>  src/compiler/glsl/ast_to_hir.cpp                   | 14 ++++
>  src/compiler/glsl/ast_type.cpp                     |  6 ++
>  src/compiler/glsl/builtin_variables.cpp            |  6 ++
>  src/compiler/glsl/glsl_parser.yy                   | 13 +++
>  src/compiler/glsl/glsl_parser_extras.cpp           |  7 ++
>  src/compiler/glsl/glsl_parser_extras.h             |  8 ++
>  src/compiler/glsl/linker.cpp                       | 25 +++++-
>  src/compiler/glsl/standalone.cpp                   |  4 +
>  src/compiler/glsl/standalone_scaffolding.cpp       |  5 ++
>  src/compiler/shader_enums.h                        |  1 +
>  src/gallium/docs/source/screen.rst                 |  4 +
>  src/gallium/drivers/ilo/ilo_screen.c               |  2 +
>  .../drivers/nouveau/codegen/nv50_ir_target.h       |  3 +-
>  src/gallium/drivers/nouveau/nv50/nv50_screen.c     |  2 +
>  src/gallium/drivers/nouveau/nvc0/nvc0_screen.c     |  1 +
>  src/gallium/drivers/radeon/r600_pipe_common.c      |  2 +
>  src/gallium/drivers/softpipe/sp_screen.c           |  1 +
>  src/gallium/include/pipe/p_defines.h               |  3 +-
>  .../glapi/gen/ARB_compute_variable_group_size.xml  | 25 ++++++
>  src/mapi/glapi/gen/Makefile.am                     |  1 +
>  src/mapi/glapi/gen/gl_API.xml                      |  4 +-
>  src/mesa/main/api_validate.c                       | 96 ++++++++++++++++++++++
>  src/mesa/main/api_validate.h                       |  4 +
>  src/mesa/main/compute.c                            | 25 ++++++
>  src/mesa/main/compute.h                            |  5 ++
>  src/mesa/main/context.c                            |  6 ++
>  src/mesa/main/dd.h                                 |  9 ++
>  src/mesa/main/extensions_table.h                   |  1 +
>  src/mesa/main/get.c                                | 12 +++
>  src/mesa/main/get_hash_params.py                   |  3 +
>  src/mesa/main/mtypes.h                             | 24 +++++-
>  src/mesa/main/shaderapi.c                          |  1 +
>  src/mesa/main/shaderobj.c                          |  2 +
>  src/mesa/main/tests/dispatch_sanity.cpp            |  3 +
>  src/mesa/state_tracker/st_cb_compute.c             | 15 +++-
>  src/mesa/state_tracker/st_extensions.c             | 22 +++++
>  src/mesa/state_tracker/st_glsl_to_tgsi.cpp         |  2 +
>  40 files changed, 365 insertions(+), 10 deletions(-)
>  create mode 100644 src/mapi/glapi/gen/ARB_compute_variable_group_size.xml
>