[Mesa-dev] [PATCH 02/78] i965/nir/vec4: Select between new nir_vec4 or current vec4_visitor code-paths
Eduardo Lima Mitev
elima at igalia.com
Mon Jun 29 14:49:38 PDT 2015
On 06/29/2015 11:22 PM, Jason Ekstrand wrote:
> On Fri, Jun 26, 2015 at 1:06 AM, Eduardo Lima Mitev <elima at 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 at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
More information about the mesa-dev
mailing list