On 01/26/2016 05:59 AM, Timothy Arceri wrote:
On Tue, 2016-01-19 at 07:20 +0100, Samuel Iglesias Gonsálvez wrote:
Commit 781d278 did not restrict consumer_stage only to separate
shader
objects, which is when we don't know if the consumer stage would be a
fragment shader added later. In normal programs, when
consumer_stage == -1, it is because they are not consumed.

Fixes 4 piglit regressions added by commit 781d278 in radeonsi:

arb_gpu_shader_fp64-double-gettransformfeedbackvarying
arb_gpu_shader_fp64-tf-interleaved
arb_gpu_shader_fp64-tf-interleaved-aligned
arb_gpu_shader_fp64-tf-separate
But what happens if you change these tests to also make use of SSO? I'm
assuming they would fall over again. That's not saying this patch isn't
correct but that the original likely wasn't a full solution.

IMO that is a different problem. SSO needs some bigger changes, like moving packing to happen only later when both consumer and producer are known.

It looks a lot like the issue I'm seeing with this [1] workaround for
SSO. Transform feedback depends on packing, it seems any change in
rules in the varying location assignment code should also be reflected
in the transform feedback code, I don't know the correct fix yet but
more piglit tests is probably the first step.

[1] http://lists.freedesktop.org/archives/mesa-dev/2016-January/105764.
html

v2:
- Simplify condition expression (Timothy).

Signed-off-by: Samuel Iglesias Gonsálvez <sigles...@igalia.com>
Tested-by: Michel Dänzer <michel.daen...@amd.com>
---
  src/glsl/link_varyings.cpp | 15 +++++++++------
  1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/src/glsl/link_varyings.cpp b/src/glsl/link_varyings.cpp
index 09f80d0..a27589d 100644
--- a/src/glsl/link_varyings.cpp
+++ b/src/glsl/link_varyings.cpp
@@ -824,7 +824,8 @@ public:
                     gl_shader_stage producer_stage,
                     gl_shader_stage consumer_stage);
     ~varying_matches();
-   void record(ir_variable *producer_var, ir_variable
*consumer_var);
+   void record(ir_variable *producer_var, ir_variable *consumer_var,
+               bool separate_shader);
     unsigned assign_locations(struct gl_shader_program *prog,
                               uint64_t reserved_slots, bool
separate_shader);
     void store_locations() const;
@@ -952,7 +953,8 @@ varying_matches::~varying_matches()
   * rendering.
   */
  void
-varying_matches::record(ir_variable *producer_var, ir_variable
*consumer_var)
+varying_matches::record(ir_variable *producer_var, ir_variable
*consumer_var,
+                        bool separate_shader)
  {
     assert(producer_var != NULL || consumer_var != NULL);
@@ -968,7 +970,8 @@ varying_matches::record(ir_variable
*producer_var, ir_variable *consumer_var)
     }
if ((consumer_var == NULL && producer_var->type-
contains_integer()) ||
-       (consumer_stage != -1 && consumer_stage !=
MESA_SHADER_FRAGMENT)) {
+       (consumer_stage != MESA_SHADER_FRAGMENT &&
+        (!separate_shader || consumer_stage != -1))) {



        /* Since this varying is not being consumed by the fragment
shader, its
         * interpolation type varying cannot possibly affect
rendering.
         * Also, this variable is non-flat and is (or contains) an
integer.
@@ -1667,7 +1670,7 @@ assign_varying_locations(struct gl_context
*ctx,
            */
           if (input_var || (prog->SeparateShader && consumer == NULL)
||
               producer->Type == GL_TESS_CONTROL_SHADER) {
-            matches.record(output_var, input_var);
+            matches.record(output_var, input_var, prog-
SeparateShader);
           }
/* Only stream 0 outputs can be consumed in the next stage
*/
@@ -1692,7 +1695,7 @@ assign_varying_locations(struct gl_context
*ctx,
               (input_var->data.mode != ir_var_shader_in))
              continue;
- matches.record(NULL, input_var);
+         matches.record(NULL, input_var, prog->SeparateShader);
        }
     }
@@ -1711,7 +1714,7 @@ assign_varying_locations(struct gl_context
*ctx,
        }
if (matched_candidate->toplevel_var-
data.is_unmatched_generic_inout)
-         matches.record(matched_candidate->toplevel_var, NULL);
+         matches.record(matched_candidate->toplevel_var, NULL, prog-
SeparateShader);
     }
const uint64_t reserved_slots =
_______________________________________________
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

Reply via email to