[Mesa-dev] [PATCH 1/2] i965: Unify shader interfaces explicitly.

[Kenneth Graunke]

A while ago, I made i965 start compiling shaders independently.  The VUE
map layouts were based entirely on each shader's input/output bitfields.
Assuming the interfaces match, this works out well - both sides will
compute the same layout, and outputs are correctly routed to inputs.

At the time, I had assumed that the linker would guarantee that the
interfaces match.  While it usually succeeds, it unfortunately seems
to fail in some cases.

For example, Piglit's tcs-input-read-array-interface test has a VS
output array with two elements, but the TCS only reads one.  The linker
isn't able to eliminate the unused element from the VS, which makes the
interfaces not match.

Another case is where a shader other than the last writes clip/cull
distances.  These should be demoted to ordinary varyings, but they
currently aren't - so we think they still have some special meaning,
and prevent them from being eliminated.

Fixing the linker to guarantee this in all cases is complicated.  It
needs to be able to optimize out dead code.  It's tied into varying
packing and other messiness.  While we can certainly improve it---and
should---I'd rather not rely on it being correct in all cases.

This patch ORs adjacent stages' input/output bitfields together,
ensuring that their interface (and hence VUE map layout) will be
compatible.  This should safeguard us against linker insufficiencies.

Fixes line rendering in Dolphin, and the Piglit test based on it:
spec/glsl-1.50/execution/geometry/clip-distance-vs-gs-out.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=97232
Signed-off-by: Kenneth Graunke <kenneth at whitecape.org>
---
 src/mesa/drivers/dri/i965/brw_link.cpp | 29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/src/mesa/drivers/dri/i965/brw_link.cpp b/src/mesa/drivers/dri/i965/brw_link.cpp
index 3f6041b..871a2fc 100644
--- a/src/mesa/drivers/dri/i965/brw_link.cpp
+++ b/src/mesa/drivers/dri/i965/brw_link.cpp
@@ -209,12 +209,32 @@ brw_new_shader(gl_shader_stage stage)
    return &shader->base;
 }
 
+static void
+unify_interfaces(struct shader_info **infos)
+{
+   struct shader_info *prev_info = NULL;
+
+   for (unsigned i = MESA_SHADER_VERTEX; i < MESA_SHADER_FRAGMENT; i++) {
+      if (!infos[i])
+         continue;
+
+      if (prev_info) {
+         prev_info->outputs_written |= infos[i]->inputs_read;
+         prev_info->patch_outputs_written |= infos[i]->patch_inputs_read;
+         infos[i]->inputs_read |= prev_info->outputs_written;
+         infos[i]->patch_inputs_read |= prev_info->patch_outputs_written;
+      }
+      prev_info = infos[i];
+   }
+}
+
 extern "C" GLboolean
 brw_link_shader(struct gl_context *ctx, struct gl_shader_program *shProg)
 {
    struct brw_context *brw = brw_context(ctx);
    const struct brw_compiler *compiler = brw->screen->compiler;
    unsigned int stage;
+   struct shader_info *infos[MESA_SHADER_STAGES] = { 0, };
 
    for (stage = 0; stage < ARRAY_SIZE(shProg->_LinkedShaders); stage++) {
       struct gl_linked_shader *shader = shProg->_LinkedShaders[stage];
@@ -269,8 +289,17 @@ brw_link_shader(struct gl_context *ctx, struct gl_shader_program *shProg)
 
       prog->nir = brw_create_nir(brw, shProg, prog, (gl_shader_stage) stage,
                                  compiler->scalar_stage[stage]);
+      infos[stage] = prog->nir->info;
    }
 
+   /* The linker tries to dead code eliminate unused varying components,
+    * and make sure interfaces match.  But it isn't able to do so in all
+    * cases.  So, explicitly make the interfaces match by OR'ing together
+    * the inputs_read/outputs_written bitfields of adjacent stages.
+    */
+   if (!shProg->SeparateShader)
+      unify_interfaces(infos);
+
    if ((ctx->_Shader->Flags & GLSL_DUMP) && shProg->Name != 0) {
       for (unsigned i = 0; i < shProg->NumShaders; i++) {
          const struct gl_shader *sh = shProg->Shaders[i];
-- 
2.10.2