[Mesa-dev] [PATCH v3 44/48] nir, intel/compiler: Use a fixed subgroup size

[Iago Toral]

On Wed, 2017-10-25 at 16:26 -0700, Jason Ekstrand wrote:
> The GL_ARB_shader_ballot spec says that gl_SubGroupSizeARB is
> declared
> as a uniform.

I was a bit surprised that we can change this so easily, then realized
that the name for this built-in is a bit misleading since this doesn't
really represent the actual size of the subgroup but the maximum size
it could possibly have and we can tell that for each shader stage of
course.

>   This means that it cannot change across an invocation
> such as a draw call or a compute dispatch.  For compute shaders,
> we're
> ok because we only ever use one dispatch size.  For fragment,
> however,
> the hardware dynamically chooses between SIMD8 and SIMD16 which
> violates
> the spec.  Instead, let's just pick a subgroup size based on the
> shader
> stage.  The fixed size we choose for compute shaders is a bit higher
> than strictly needed but there's no real harm in that.  The advantage
> is
> that, if they do anything interesting with the value, NIR will see it
> as
> an immediate and can optimize better.
> 
> Acked-by: Lionel Landwerlin <lionel.g.landwerlin at intel.com>
> ---
>  src/compiler/nir/nir.h                 | 1 +
>  src/compiler/nir/nir_lower_subgroups.c | 5 +++++
>  src/intel/compiler/brw_fs_nir.cpp      | 4 ----
>  src/intel/compiler/brw_nir.c           | 2 ++
>  4 files changed, 8 insertions(+), 4 deletions(-)
> 
> diff --git a/src/compiler/nir/nir.h b/src/compiler/nir/nir.h
> index 563b57f..df18bfa 100644
> --- a/src/compiler/nir/nir.h
> +++ b/src/compiler/nir/nir.h
> @@ -2467,6 +2467,7 @@ bool nir_lower_samplers_as_deref(nir_shader
> *shader,
>                                   const struct gl_shader_program
> *shader_program);
>  
>  typedef struct nir_lower_subgroups_options {
> +   uint8_t subgroup_size;
>     uint8_t ballot_bit_size;
>     bool lower_to_scalar:1;
>     bool lower_vote_trivial:1;
> diff --git a/src/compiler/nir/nir_lower_subgroups.c
> b/src/compiler/nir/nir_lower_subgroups.c
> index 1969740..f9424c9 100644
> --- a/src/compiler/nir/nir_lower_subgroups.c
> +++ b/src/compiler/nir/nir_lower_subgroups.c
> @@ -109,6 +109,11 @@ lower_subgroups_intrin(nir_builder *b,
> nir_intrinsic_instr *intrin,
>           return nir_imm_int(b, NIR_TRUE);
>        break;
>  
> +   case nir_intrinsic_load_subgroup_size:
> +      if (options->subgroup_size)
> +         return nir_imm_int(b, options->subgroup_size);
> +      break;
> +
>     case nir_intrinsic_read_invocation:
>     case nir_intrinsic_read_first_invocation:
>        if (options->lower_to_scalar)
> diff --git a/src/intel/compiler/brw_fs_nir.cpp
> b/src/intel/compiler/brw_fs_nir.cpp
> index b552387..47730e1 100644
> --- a/src/intel/compiler/brw_fs_nir.cpp
> +++ b/src/intel/compiler/brw_fs_nir.cpp
> @@ -4183,10 +4183,6 @@ fs_visitor::nir_emit_intrinsic(const
> fs_builder &bld, nir_intrinsic_instr *instr
>        break;
>     }
>  
> -   case nir_intrinsic_load_subgroup_size:
> -      bld.MOV(retype(dest, BRW_REGISTER_TYPE_D),
> brw_imm_d(dispatch_width));
> -      break;
> -
>     case nir_intrinsic_load_subgroup_invocation:
>        bld.MOV(retype(dest, BRW_REGISTER_TYPE_D),
>                nir_system_values[SYSTEM_VALUE_SUBGROUP_INVOCATION]);
> diff --git a/src/intel/compiler/brw_nir.c
> b/src/intel/compiler/brw_nir.c
> index 0d59d36..5ed36fe 100644
> --- a/src/intel/compiler/brw_nir.c
> +++ b/src/intel/compiler/brw_nir.c
> @@ -637,6 +637,8 @@ brw_preprocess_nir(const struct brw_compiler
> *compiler, nir_shader *nir)
>     OPT(nir_lower_system_values);
>  
>     const nir_lower_subgroups_options subgroups_options = {
> +      .subgroup_size = nir->info.stage == MESA_SHADER_COMPUTE ? 32 :
> +                       nir->info.stage == MESA_SHADER_FRAGMENT ? 16
> : 8,
>        .ballot_bit_size = 32,
>        .lower_to_scalar = true,
>        .lower_subgroup_masks = true,