brw_link_shader is also the responsible to create the NIR. In the case
of GLSL it uses the IR (process ir + glsl_to_nir), in the case of
ARB_gl_spirv it uses the spir-v shader (some process + spirv_to_nir).

After getting the NIR, it applies some lowerings.

Until now, ARB_gl_spirv implementation was assuming that those
lowerings were "pure-NIR", so it started the linking based on NIR
after brw_create_nir.

But since commit "anv,i965: Lower away image derefs in the driver"
brw_create_nir calls brw_nir_lower_glsl_images that assumes that the
UniformStorage was already initialized, so that the uniform linking
was already done, in order to know the sampler idx.

This pending-to-be cleaned patch just moves the call of this
lowering. Now it is called after gl_nir_link_uniforms. As far as we
see, that fixed all regressions on ARB_gl_spirv tests.

But:

  * Perhaps it is not really correct to do that. After all,
    gl_nir_link_uniforms links based on the nir shader, and the
    correct would be do the linking after that lowering. In that case
    we would need to compute the sampler_idx in an alternative way.

  * If it is ok to move the lowering for the ARB_gl_spirv case,
    perhaps it is ok to move it on the GLSL case. In that case the
    solution will be more simple and clean.

Opinions?

Finally, the lowering is called brw_nir_lower_glsl_images. But looking
a little to it, except for the uniform storage thing, I don't see any
reason to not use it on the ARB_gl_spirv codepath. So perhaps it would
be worthy to rename it to brw_nir_lower_opengl_images or
brw_nir_lower_gl_images?
---
 src/mesa/drivers/dri/i965/brw_link.cpp  | 11 +++++++++++
 src/mesa/drivers/dri/i965/brw_program.c |  8 ++++++--
 2 files changed, 17 insertions(+), 2 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/brw_link.cpp 
b/src/mesa/drivers/dri/i965/brw_link.cpp
index 0723255dec6..70375ca3f29 100644
--- a/src/mesa/drivers/dri/i965/brw_link.cpp
+++ b/src/mesa/drivers/dri/i965/brw_link.cpp
@@ -268,6 +268,17 @@ brw_link_shader(struct gl_context *ctx, struct 
gl_shader_program *shProg)
 
       gl_nir_link_assign_atomic_counter_resources(ctx, shProg);
       gl_nir_link_assign_xfb_resources(ctx, shProg);
+
+      for (unsigned stage = 0; stage < ARRAY_SIZE(shProg->_LinkedShaders); 
stage++) {
+         struct gl_linked_shader *shader = shProg->_LinkedShaders[stage];
+         if (!shader)
+            continue;
+
+         struct gl_program *prog = shader->Program;
+         prog->Parameters = _mesa_new_parameter_list();
+
+         NIR_PASS_V(prog->nir, brw_nir_lower_glsl_images, prog);
+      }
    }
 
    /* Determine first and last stage. */
diff --git a/src/mesa/drivers/dri/i965/brw_program.c 
b/src/mesa/drivers/dri/i965/brw_program.c
index 041395ec4c0..68f6e18bb09 100644
--- a/src/mesa/drivers/dri/i965/brw_program.c
+++ b/src/mesa/drivers/dri/i965/brw_program.c
@@ -139,8 +139,12 @@ brw_create_nir(struct brw_context *brw,
       }
    }
 
-   NIR_PASS_V(nir, brw_nir_lower_uniforms, is_scalar);
-   NIR_PASS_V(nir, brw_nir_lower_glsl_images, prog);
+   if (shader_prog && shader_prog->data->spirv) {
+      NIR_PASS_V(nir, brw_nir_lower_uniforms, is_scalar);
+   } else {
+      NIR_PASS_V(nir, brw_nir_lower_uniforms, is_scalar);
+      NIR_PASS_V(nir, brw_nir_lower_glsl_images, prog);
+   }
 
    return nir;
 }
-- 
2.14.1

_______________________________________________
mesa-dev mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Reply via email to