On Mon, Jun 29, 2015 at 2:49 PM, Eduardo Lima Mitev <el...@igalia.com> wrote:
> 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:
Right... Sorry for the noise.
--Jason
>>> + }
>>> 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
