[Mesa-dev] [PATCH 02/78] i965/nir/vec4: Select between new nir_vec4 or current vec4_visitor code-paths

[Kenneth Graunke]

On Friday, June 26, 2015 10:06:18 AM Eduardo Lima Mitev wrote:
> The NIR->vec4 pass will be activated if ALL the following conditions are met:
> 
> * INTEL_USE_NIR environment variable is defined and is positive (1 or true)
> * The stage is vertex shader
> * The HW generation is either SandyBridge (gen6), IvyBridge or Haswell (gen7)
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=89580
> ---
>  src/mesa/drivers/dri/i965/brw_program.c  |  5 +++++
>  src/mesa/drivers/dri/i965/brw_shader.cpp | 14 ++++++++------
>  src/mesa/drivers/dri/i965/brw_vec4.cpp   | 32 ++++++++++++++++++++++++++++++--
>  src/mesa/drivers/dri/i965/brw_vec4.h     |  2 ++
>  4 files changed, 45 insertions(+), 8 deletions(-)
> 
> diff --git a/src/mesa/drivers/dri/i965/brw_program.c b/src/mesa/drivers/dri/i965/brw_program.c
> index 2327af7..7e5d23d 100644
> --- a/src/mesa/drivers/dri/i965/brw_program.c
> +++ b/src/mesa/drivers/dri/i965/brw_program.c
> @@ -574,6 +574,11 @@ brw_dump_ir(const char *stage, struct gl_shader_program *shader_prog,
>              struct gl_shader *shader, struct gl_program *prog)
>  {
>     if (shader_prog) {
> +      /* Since git~104c8fc, shader->ir can be NULL if NIR is used.
> +       * That must have been checked prior to calling this function, but
> +       * we double-check here just in case.
> +       */

That got reverted, so you can probably drop this comment.  The assertion
seems reasonable.

> +      assert(shader->ir != NULL);
>        fprintf(stderr,
>                "GLSL IR for native %s shader %d:\n", stage, shader_prog->Name);
>        _mesa_print_ir(stderr, shader->ir, NULL);
> diff --git a/src/mesa/drivers/dri/i965/brw_shader.cpp b/src/mesa/drivers/dri/i965/brw_shader.cpp
> index 5653d6b..0b53647 100644
> --- a/src/mesa/drivers/dri/i965/brw_shader.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_shader.cpp
> @@ -118,12 +118,14 @@ brw_compiler_create(void *mem_ctx, const struct brw_device_info *devinfo)
>     compiler->glsl_compiler_options[MESA_SHADER_VERTEX].OptimizeForAOS = true;
>     compiler->glsl_compiler_options[MESA_SHADER_GEOMETRY].OptimizeForAOS = true;
>  
> -   if (compiler->scalar_vs) {
> -      /* If we're using the scalar backend for vertex shaders, we need to
> -       * configure these accordingly.
> -       */
> -      compiler->glsl_compiler_options[MESA_SHADER_VERTEX].EmitNoIndirectOutput = true;
> -      compiler->glsl_compiler_options[MESA_SHADER_VERTEX].EmitNoIndirectTemp = true;
> +   if (compiler->scalar_vs || brw_env_var_as_boolean("INTEL_USE_NIR", false)) {
> +      if (compiler->scalar_vs) {
> +         /* If we're using the scalar backend for vertex shaders, we need to
> +         * configure these accordingly.
> +         */

indentation looks a bit off here.

> +         compiler->glsl_compiler_options[MESA_SHADER_VERTEX].EmitNoIndirectOutput = true;
> +         compiler->glsl_compiler_options[MESA_SHADER_VERTEX].EmitNoIndirectTemp = true;
> +      }
>        compiler->glsl_compiler_options[MESA_SHADER_VERTEX].OptimizeForAOS = false;
>  
>        compiler->glsl_compiler_options[MESA_SHADER_VERTEX].NirOptions = nir_options;
> diff --git a/src/mesa/drivers/dri/i965/brw_vec4.cpp b/src/mesa/drivers/dri/i965/brw_vec4.cpp
> index a5c686c..dcffa04 100644
> --- a/src/mesa/drivers/dri/i965/brw_vec4.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_vec4.cpp
> @@ -1707,6 +1707,21 @@ vec4_visitor::emit_shader_time_write(int shader_time_subindex, src_reg value)
>  }
>  
>  bool
> +vec4_visitor::should_use_vec4_nir()
> +{
> +   /* NIR->vec4 pass is activated when all these conditions meet:
> +    *
> +    * 1) it is a vertex shader
> +    * 2) INTEL_USE_NIR env-var set to true, so NirOptions are defined for VS
> +    * 3) hardware gen is SNB, IVB or HSW
> +    */
> +   return
> +      stage == MESA_SHADER_VERTEX &&
> +      compiler->glsl_compiler_options[MESA_SHADER_VERTEX].NirOptions != NULL &&
> +      devinfo->gen >= 6 && devinfo->gen < 8;

Why not just do:

   return compiler->glsl_compiler_options[stage].NirOptions != NULL;

As long as you don't set NirOptions for geometry shaders, that will
work fine.  You could also only set NirOptions[MESA_SHADER_VERTEX] when
gen >= 6 && gen < 8, if you like.  You could probably just eliminate the
function at that point.

> +}
> +
> +bool
>  vec4_visitor::run(gl_clip_plane *clip_planes)
>  {
>     sanity_param_count = prog->Parameters->NumParameters;
> @@ -1722,7 +1737,17 @@ vec4_visitor::run(gl_clip_plane *clip_planes)
>      * functions called "main").
>      */
>     if (shader) {
> -      visit_instructions(shader->base.ir);
> +      if (should_use_vec4_nir()) {
> +         assert(prog->nir != NULL);
> +         emit_nir_code();
> +         if (failed)
> +            return false;
> +      } else {
> +         /* Generate VS IR for main().  (the visitor only descends into
> +          * functions called "main").
> +          */
> +         visit_instructions(shader->base.ir);
> +      }
>     } else {
>        emit_program_code();
>     }
> @@ -1882,7 +1907,10 @@ brw_vs_emit(struct brw_context *brw,
>        st_index = brw_get_shader_time_index(brw, prog, &c->vp->program.Base,
>                                             ST_VS);
>  
> -   if (unlikely(INTEL_DEBUG & DEBUG_VS))
> +   /* Since git~104c8fc, shader->base.ir can be NULL if NIR is used,
> +    * so we need to check that before dumping IR tree.
> +    */

That got reverted, so you can probably drop this hunk.

> +   if (unlikely(INTEL_DEBUG & DEBUG_VS) && (!prog || shader->base.ir))
>        brw_dump_ir("vertex", prog, &shader->base, &c->vp->program.Base);
>  
>     if (brw->intelScreen->compiler->scalar_vs) {
> diff --git a/src/mesa/drivers/dri/i965/brw_vec4.h b/src/mesa/drivers/dri/i965/brw_vec4.h
> index 4f2cc86..68bc4ae 100644
> --- a/src/mesa/drivers/dri/i965/brw_vec4.h
> +++ b/src/mesa/drivers/dri/i965/brw_vec4.h
> @@ -389,6 +389,8 @@ public:
>  
>     void visit_atomic_counter_intrinsic(ir_call *ir);
>  
> +   virtual bool should_use_vec4_nir();
> +
>     virtual void emit_nir_code();
>     virtual void nir_setup_inputs(nir_shader *shader);
>     virtual void nir_setup_outputs(nir_shader *shader);
> 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20150629/68fc488d/attachment.sig>