On 06/29/2015 11:22 PM, Jason Ekstrand wrote: > On Fri, Jun 26, 2015 at 1:06 AM, Eduardo Lima Mitev <el...@igalia.com> 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) > > I'm not sure about this last one. When we did this for FS, it was > well-known that HSW and IVB were the only ones that were working if > you used INTEL_USE_NIR on Iron Lake, you got what you got. This makes > it easier to develop/test on older platforms because it doesn't > involve hacking things up. > > Only using it for vertex shaders is perfectly reasonable because there > are whole chunks of stuff for geometry that simply doesn't exist yet. >
Ok, I will drop that condition then, and perhaps spit a warning if gen<6 to alert that NIR->vec4 doesn't yet support that gen, so crashes might happen. >> >> 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. >> + */ >> + 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. >> + */ >> + >> compiler->glsl_compiler_options[MESA_SHADER_VERTEX].EmitNoIndirectOutput = >> true; >> + >> compiler->glsl_compiler_options[MESA_SHADER_VERTEX].EmitNoIndirectTemp = >> true; > > This seems wrong. The vec4 backend can certainly handle indirect > temporaries and indirect outputs are a must for geometry shaders. I > really think we only want to turn these on for scalar shaders. > Actually, these two settings do not get executed for our pass, because compiler->scalar_vs is still false. Notice the OR in the outer-most if. What this patch chunk effectively does is to execute the next two lines below here, for our nir-vec4 pass: >> + } >> 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; >> +} >> + >> +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. >> + */ >> + 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); >> -- >> 2.1.4 >> >> _______________________________________________ >> mesa-dev mailing list >> mesa-dev@lists.freedesktop.org >> http://lists.freedesktop.org/mailman/listinfo/mesa-dev > _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
