Re: [Mesa-dev] [PATCH] intel/compiler: Make brw_nir_lower_intrinsics compute-specific

[Jordan Justen]

Reviewed-by: Jordan Justen 

On 2017-10-06 10:20:28, Jason Ekstrand wrote:
> It's already only ever called from brw_compile_cs and only handles
> compute intrinsics.  Let's just make it CS-specific.  We can always
> make it handle other stages again later if we want.
> 
> Cc: Jordan Justen 
> 
> ---
>  src/intel/Makefile.sources |  2 +-
>  src/intel/compiler/brw_fs.cpp  |  2 +-
>  src/intel/compiler/brw_nir.h   |  4 ++--
>  ..._intrinsics.c => brw_nir_lower_cs_intrinsics.c} | 22 
> --
>  4 files changed, 12 insertions(+), 18 deletions(-)
>  rename src/intel/compiler/{brw_nir_intrinsics.c => 
> brw_nir_lower_cs_intrinsics.c} (89%)
> 
> diff --git a/src/intel/Makefile.sources b/src/intel/Makefile.sources
> index 9672dcc..b835533 100644
> --- a/src/intel/Makefile.sources
> +++ b/src/intel/Makefile.sources
> @@ -75,7 +75,7 @@ COMPILER_FILES = \
> compiler/brw_nir_analyze_boolean_resolves.c \
> compiler/brw_nir_analyze_ubo_ranges.c \
> compiler/brw_nir_attribute_workarounds.c \
> -   compiler/brw_nir_intrinsics.c \
> +   compiler/brw_nir_lower_cs_intrinsics.c \
> compiler/brw_nir_opt_peephole_ffma.c \
> compiler/brw_nir_tcs_workarounds.c \
> compiler/brw_packed_float.c \
> diff --git a/src/intel/compiler/brw_fs.cpp b/src/intel/compiler/brw_fs.cpp
> index 6f5f21d..371df71 100644
> --- a/src/intel/compiler/brw_fs.cpp
> +++ b/src/intel/compiler/brw_fs.cpp
> @@ -6766,7 +6766,7 @@ brw_compile_cs(const struct brw_compiler *compiler, 
> void *log_data,
>MAX2(shader->num_uniforms,
> (unsigned)4 * (prog_data->thread_local_id_index + 1));
>  
> -   brw_nir_lower_intrinsics(shader, _data->base);
> +   brw_nir_lower_cs_intrinsics(shader, prog_data);
> shader = brw_postprocess_nir(shader, compiler, true);
>  
> prog_data->local_size[0] = shader->info.cs.local_size[0];
> diff --git a/src/intel/compiler/brw_nir.h b/src/intel/compiler/brw_nir.h
> index f4b13b1..6f64a73 100644
> --- a/src/intel/compiler/brw_nir.h
> +++ b/src/intel/compiler/brw_nir.h
> @@ -95,8 +95,8 @@ void brw_nir_analyze_boolean_resolves(nir_shader *nir);
>  nir_shader *brw_preprocess_nir(const struct brw_compiler *compiler,
> nir_shader *nir);
>  
> -bool brw_nir_lower_intrinsics(nir_shader *nir,
> -  struct brw_stage_prog_data *prog_data);
> +bool brw_nir_lower_cs_intrinsics(nir_shader *nir,
> + struct brw_cs_prog_data *prog_data);
>  void brw_nir_lower_vs_inputs(nir_shader *nir,
>   bool use_legacy_snorm_formula,
>   const uint8_t *vs_attrib_wa_flags);
> diff --git a/src/intel/compiler/brw_nir_intrinsics.c 
> b/src/intel/compiler/brw_nir_lower_cs_intrinsics.c
> similarity index 89%
> rename from src/intel/compiler/brw_nir_intrinsics.c
> rename to src/intel/compiler/brw_nir_lower_cs_intrinsics.c
> index abbbc6f..602ef2e 100644
> --- a/src/intel/compiler/brw_nir_intrinsics.c
> +++ b/src/intel/compiler/brw_nir_lower_cs_intrinsics.c
> @@ -26,10 +26,7 @@
>  
>  struct lower_intrinsics_state {
> nir_shader *nir;
> -   union {
> -  struct brw_stage_prog_data *prog_data;
> -  struct brw_cs_prog_data *cs_prog_data;
> -   };
> +   struct brw_cs_prog_data *prog_data;
> nir_function_impl *impl;
> bool progress;
> nir_builder builder;
> @@ -50,9 +47,9 @@ read_thread_local_id(struct lower_intrinsics_state *state)
> if (group_size <= 8)
>return nir_imm_int(b, 0);
>  
> -   assert(state->cs_prog_data->thread_local_id_index >= 0);
> +   assert(state->prog_data->thread_local_id_index >= 0);
> state->cs_thread_id_used = true;
> -   const int id_index = state->cs_prog_data->thread_local_id_index;
> +   const int id_index = state->prog_data->thread_local_id_index;
>  
> nir_intrinsic_instr *load =
>nir_intrinsic_instr_create(nir, nir_intrinsic_load_uniform);
> @@ -84,7 +81,6 @@ lower_cs_intrinsics_convert_block(struct 
> lower_intrinsics_state *state,
>nir_ssa_def *sysval;
>switch (intrinsic->intrinsic) {
>case nir_intrinsic_load_local_invocation_index: {
> - assert(nir->stage == MESA_SHADER_COMPUTE);
>   /* We construct the local invocation index from:
>*
>*gl_LocalInvocationIndex =
> @@ -97,7 +93,6 @@ lower_cs_intrinsics_convert_block(struct 
> lower_intrinsics_state *state,
>}
>  
>case nir_intrinsic_load_local_invocation_id: {
> - assert(nir->stage == MESA_SHADER_COMPUTE);
>   /* We lower gl_LocalInvocationID from gl_LocalInvocationIndex based
>* on this formula:
>*
> @@ -156,11 +151,10 @@ lower_cs_intrinsics_convert_impl(struct 
> lower_intrinsics_state *state)
>  }
>  
>  bool
> -brw_nir_lower_intrinsics(nir_shader *nir, struct 

[Mesa-dev] [PATCH] intel/compiler: Make brw_nir_lower_intrinsics compute-specific

[Jason Ekstrand]

It's already only ever called from brw_compile_cs and only handles
compute intrinsics.  Let's just make it CS-specific.  We can always
make it handle other stages again later if we want.

Cc: Jordan Justen 

---
 src/intel/Makefile.sources |  2 +-
 src/intel/compiler/brw_fs.cpp  |  2 +-
 src/intel/compiler/brw_nir.h   |  4 ++--
 ..._intrinsics.c => brw_nir_lower_cs_intrinsics.c} | 22 --
 4 files changed, 12 insertions(+), 18 deletions(-)
 rename src/intel/compiler/{brw_nir_intrinsics.c => 
brw_nir_lower_cs_intrinsics.c} (89%)

diff --git a/src/intel/Makefile.sources b/src/intel/Makefile.sources
index 9672dcc..b835533 100644
--- a/src/intel/Makefile.sources
+++ b/src/intel/Makefile.sources
@@ -75,7 +75,7 @@ COMPILER_FILES = \
compiler/brw_nir_analyze_boolean_resolves.c \
compiler/brw_nir_analyze_ubo_ranges.c \
compiler/brw_nir_attribute_workarounds.c \
-   compiler/brw_nir_intrinsics.c \
+   compiler/brw_nir_lower_cs_intrinsics.c \
compiler/brw_nir_opt_peephole_ffma.c \
compiler/brw_nir_tcs_workarounds.c \
compiler/brw_packed_float.c \
diff --git a/src/intel/compiler/brw_fs.cpp b/src/intel/compiler/brw_fs.cpp
index 6f5f21d..371df71 100644
--- a/src/intel/compiler/brw_fs.cpp
+++ b/src/intel/compiler/brw_fs.cpp
@@ -6766,7 +6766,7 @@ brw_compile_cs(const struct brw_compiler *compiler, void 
*log_data,
   MAX2(shader->num_uniforms,
(unsigned)4 * (prog_data->thread_local_id_index + 1));
 
-   brw_nir_lower_intrinsics(shader, _data->base);
+   brw_nir_lower_cs_intrinsics(shader, prog_data);
shader = brw_postprocess_nir(shader, compiler, true);
 
prog_data->local_size[0] = shader->info.cs.local_size[0];
diff --git a/src/intel/compiler/brw_nir.h b/src/intel/compiler/brw_nir.h
index f4b13b1..6f64a73 100644
--- a/src/intel/compiler/brw_nir.h
+++ b/src/intel/compiler/brw_nir.h
@@ -95,8 +95,8 @@ void brw_nir_analyze_boolean_resolves(nir_shader *nir);
 nir_shader *brw_preprocess_nir(const struct brw_compiler *compiler,
nir_shader *nir);
 
-bool brw_nir_lower_intrinsics(nir_shader *nir,
-  struct brw_stage_prog_data *prog_data);
+bool brw_nir_lower_cs_intrinsics(nir_shader *nir,
+ struct brw_cs_prog_data *prog_data);
 void brw_nir_lower_vs_inputs(nir_shader *nir,
  bool use_legacy_snorm_formula,
  const uint8_t *vs_attrib_wa_flags);
diff --git a/src/intel/compiler/brw_nir_intrinsics.c 
b/src/intel/compiler/brw_nir_lower_cs_intrinsics.c
similarity index 89%
rename from src/intel/compiler/brw_nir_intrinsics.c
rename to src/intel/compiler/brw_nir_lower_cs_intrinsics.c
index abbbc6f..602ef2e 100644
--- a/src/intel/compiler/brw_nir_intrinsics.c
+++ b/src/intel/compiler/brw_nir_lower_cs_intrinsics.c
@@ -26,10 +26,7 @@
 
 struct lower_intrinsics_state {
nir_shader *nir;
-   union {
-  struct brw_stage_prog_data *prog_data;
-  struct brw_cs_prog_data *cs_prog_data;
-   };
+   struct brw_cs_prog_data *prog_data;
nir_function_impl *impl;
bool progress;
nir_builder builder;
@@ -50,9 +47,9 @@ read_thread_local_id(struct lower_intrinsics_state *state)
if (group_size <= 8)
   return nir_imm_int(b, 0);
 
-   assert(state->cs_prog_data->thread_local_id_index >= 0);
+   assert(state->prog_data->thread_local_id_index >= 0);
state->cs_thread_id_used = true;
-   const int id_index = state->cs_prog_data->thread_local_id_index;
+   const int id_index = state->prog_data->thread_local_id_index;
 
nir_intrinsic_instr *load =
   nir_intrinsic_instr_create(nir, nir_intrinsic_load_uniform);
@@ -84,7 +81,6 @@ lower_cs_intrinsics_convert_block(struct 
lower_intrinsics_state *state,
   nir_ssa_def *sysval;
   switch (intrinsic->intrinsic) {
   case nir_intrinsic_load_local_invocation_index: {
- assert(nir->stage == MESA_SHADER_COMPUTE);
  /* We construct the local invocation index from:
   *
   *gl_LocalInvocationIndex =
@@ -97,7 +93,6 @@ lower_cs_intrinsics_convert_block(struct 
lower_intrinsics_state *state,
   }
 
   case nir_intrinsic_load_local_invocation_id: {
- assert(nir->stage == MESA_SHADER_COMPUTE);
  /* We lower gl_LocalInvocationID from gl_LocalInvocationIndex based
   * on this formula:
   *
@@ -156,11 +151,10 @@ lower_cs_intrinsics_convert_impl(struct 
lower_intrinsics_state *state)
 }
 
 bool
-brw_nir_lower_intrinsics(nir_shader *nir, struct brw_stage_prog_data 
*prog_data)
+brw_nir_lower_cs_intrinsics(nir_shader *nir,
+struct brw_cs_prog_data *prog_data)
 {
-   /* Currently we only lower intrinsics for compute shaders */
-   if (nir->stage != MESA_SHADER_COMPUTE)
-  return false;
+   assert(nir->stage == MESA_SHADER_COMPUTE);
 
bool