From: Kenneth Graunke Date: Mon, 5 Dec 2016 07:54:48 +0000 (-0800) Subject: i965: Unify shader interfaces explicitly. X-Git-Url: https://git.libre-soc.org/?a=commitdiff_plain;h=44fd85d8eb1fba68829917c0cf5ce052095964ee;p=mesa.git i965: Unify shader interfaces explicitly. 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 Reviewed-by: Timothy Arceri --- diff --git a/src/mesa/drivers/dri/i965/brw_link.cpp b/src/mesa/drivers/dri/i965/brw_link.cpp index 3f6041b7ba2..871a2fc1a9a 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];