On 06/29/2015 10:20 PM, Kenneth Graunke wrote: > 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. >
Ok, I will remove the comment then. There is also a patch from Neil Roberts in the mailing list <http://lists.freedesktop.org/archives/mesa-dev/2015-June/087607.html> that handles this by checking for shader->ir null-ness. Maybe that's just enough and the assert is not needed. >> + 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. > Yes, you that's right. I'm setting NirOptions only to vertex shaders' compiler options; and together with Jason suggestion of removing the gen check, we are left with just a check for NirOptions. I will change that and remove the method too. >> +} + +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); >> _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
