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

[Eduardo Lima Mitev]

On 07/23/2015 11:25 PM, Jason Ekstrand wrote:
> On Thu, Jul 23, 2015 at 3:16 AM, Eduardo Lima Mitev <elima at igalia.com> wrote:
>> The NIR->vec4 pass will be activated if both the following conditions are met:
>>
>> * INTEL_USE_NIR environment variable is defined and is positive (1 or true)
>> * The stage is vertex shader (support for geometry shaders and
>>   ARB_vertex_program will be added later).
>> ---
>>  src/mesa/drivers/dri/i965/brw_shader.cpp | 14 ++++++++------
>>  src/mesa/drivers/dri/i965/brw_vec4.cpp   | 18 ++++++++++++++----
>>  2 files changed, 22 insertions(+), 10 deletions(-)
>>
>> diff --git a/src/mesa/drivers/dri/i965/brw_shader.cpp b/src/mesa/drivers/dri/i965/brw_shader.cpp
>> index 9d60543..cb04d8a 100644
>> --- a/src/mesa/drivers/dri/i965/brw_shader.cpp
>> +++ b/src/mesa/drivers/dri/i965/brw_shader.cpp
>> @@ -122,12 +122,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;
>> +      }
>>        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 53270fb..ce04f1b 100644
>> --- a/src/mesa/drivers/dri/i965/brw_vec4.cpp
>> +++ b/src/mesa/drivers/dri/i965/brw_vec4.cpp
>> @@ -1709,6 +1709,9 @@ vec4_visitor::emit_shader_time_write(int shader_time_subindex, src_reg value)
>>  bool
>>  vec4_visitor::run(gl_clip_plane *clip_planes)
>>  {
>> +   bool use_vec4_nir =
>> +      compiler->glsl_compiler_options[MESA_SHADER_VERTEX].NirOptions != NULL;
>> +
>>     sanity_param_count = prog->Parameters->NumParameters;
>>
>>     if (shader_time_index >= 0)
>> @@ -1718,11 +1721,18 @@ vec4_visitor::run(gl_clip_plane *clip_planes)
>>
>>     emit_prolog();
>>
>> -   /* Generate VS IR for main().  (the visitor only descends into
>> -    * functions called "main").
>> -    */
>>     if (shader) {
>> -      visit_instructions(shader->base.ir);
>> +      if (use_vec4_nir) {
> 
> We could put the compiler_options[].NirOptions check here.  I don't
> care too much though.
> 

Yes, the extra variable is not necessary at this point, but later on in
the series we have the patch
http://lists.freedesktop.org/archives/mesa-dev/2015-July/089653.html
which uses the bool variable again in the same method. Hence, I think we
can leave it there if you don't feel strongly about it. Otherwise, we
can introduce the variable in the later commit where the condition is
needed again.

>> +         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();
>>     }
>> --
>> 2.1.4
>>
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>