Hi Ken,

I found that this patch causes a regression. There's a Windows medical app which fails to link some shaders since this change.


Basically, when the gl_Position VS input is declared as invariant the linker fails with:

error: declarations for uniform `gl_ModelViewProjectionMatrix' have mismatching invariant qualifiers

I haven't investigated how to fix this. I'm hoping you can see a simple fix.

The attached piglit shader_runner script demonstrates the issue. Passes w/ NVIDIA.

Thanks!

-Brian



On 09/23/2016 05:45 PM, Kenneth Graunke wrote:
Module: Mesa
Branch: master
Commit: b04ef3c08a288a5857349c9e582ee2718fa562f7
URL:    
https://urldefense.proofpoint.com/v2/url?u=http-3A__cgit.freedesktop.org_mesa_mesa_commit_-3Fid-3Db04ef3c08a288a5857349c9e582ee2718fa562f7&d=CwIGaQ&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=T0t4QG7chq2ZwJo6wilkFznRSFy-8uDKartPGbomVj8&m=m7lwXZjH2_UAMD5u1FWrl6EmaAyly794Od4UBt09XC4&s=k1P13rDoBzgIU78tLDWd_Qo9GTSr_IX2GSRtdfMrDeI&e=

Author: Kenneth Graunke <kenn...@whitecape.org>
Date:   Fri May 30 23:52:22 2014 -0700

glsl: Immediately inline built-ins rather than generating calls.

In the past, we imported the prototypes of built-in functions, generated
calls to those, and waited until link time to resolve the calls and
import the actual code for the built-in functions.

This severely limited our compile-time optimization opportunities: even
trivial functions like dot() were represented as function calls.  We
also had no way of reasoning about those calls; they could have been
1,000 line functions with side-effects for all we knew.

Practically all built-in functions are trivial translations to
ir_expression opcodes, so it makes sense to just generate those inline.
Since we eventually inline all functions anyway, we may as well just do
it for all built-in functions.

There's only one snag: built-in functions that refer to built-in global
variables need those remapped to the variables in the shader being
compiled, rather than the ones in the built-in shader.  Currently,
ftransform() is the only function matching those criteria, so it seemed
easier to just make it a special case.

On Skylake:

total instructions in shared programs: 12023491 -> 12024010 (0.00%)
instructions in affected programs: 77595 -> 78114 (0.67%)
helped: 97
HURT: 309

total cycles in shared programs: 137239044 -> 137295498 (0.04%)
cycles in affected programs: 16714026 -> 16770480 (0.34%)
helped: 4663
HURT: 4923

while these statistics are in the wrong direction, the number of
hurt programs is small (309 / 41282 = 0.75%), and I don't think
anything can be done about it.  A change like this significantly
alters the order in which optimizations are performed.

Signed-off-by: Kenneth Graunke <kenn...@whitecape.org>
Reviewed-by; Ian Romanick <ian.d.roman...@intel.com>

---

  src/compiler/glsl/ast_function.cpp | 46 ++++++++++++++++++--------------------
  1 file changed, 22 insertions(+), 24 deletions(-)

diff --git a/src/compiler/glsl/ast_function.cpp 
b/src/compiler/glsl/ast_function.cpp
index 7e62ab7..ac3b52d 100644
--- a/src/compiler/glsl/ast_function.cpp
+++ b/src/compiler/glsl/ast_function.cpp
@@ -430,7 +430,8 @@ generate_call(exec_list *instructions, 
ir_function_signature *sig,
                exec_list *actual_parameters,
                ir_variable *sub_var,
                ir_rvalue *array_idx,
-              struct _mesa_glsl_parse_state *state)
+              struct _mesa_glsl_parse_state *state,
+              bool inline_immediately)
  {
     void *ctx = state;
     exec_list post_call_conversions;
@@ -542,6 +543,10 @@ generate_call(exec_list *instructions, 
ir_function_signature *sig,
     ir_call *call = new(ctx) ir_call(sig, deref,
                                      actual_parameters, sub_var, array_idx);
     instructions->push_tail(call);
+   if (inline_immediately) {
+      call->generate_inline(call);
+      call->remove();
+   }

     /* Also emit any necessary out-parameter conversions. */
     instructions->append_list(&post_call_conversions);
@@ -557,19 +562,18 @@ match_function_by_name(const char *name,
                         exec_list *actual_parameters,
                         struct _mesa_glsl_parse_state *state)
  {
-   void *ctx = state;
     ir_function *f = state->symbols->get_function(name);
     ir_function_signature *local_sig = NULL;
     ir_function_signature *sig = NULL;

     /* Is the function hidden by a record type constructor? */
     if (state->symbols->get_type(name))
-      goto done; /* no match */
+      return sig; /* no match */

     /* Is the function hidden by a variable (impossible in 1.10)? */
     if (!state->symbols->separate_function_namespace
         && state->symbols->get_variable(name))
-      goto done; /* no match */
+      return sig; /* no match */

     if (f != NULL) {
        /* In desktop GL, the presence of a user-defined signature hides any
@@ -583,31 +587,15 @@ match_function_by_name(const char *name,
        sig = local_sig = f->matching_signature(state, actual_parameters,
                                                allow_builtins, &is_exact);
        if (is_exact)
-         goto done;
+         return sig;

        if (!allow_builtins)
-         goto done;
+         return sig;
     }

     /* Local shader has no exact candidates; check the built-ins. */
     _mesa_glsl_initialize_builtin_functions();
     sig = _mesa_glsl_find_builtin_function(state, name, actual_parameters);
-
- done:
-   if (sig != NULL) {
-      /* If the match is from a linked built-in shader, import the
-       * prototype.
-       */
-      if (sig != local_sig) {
-         if (f == NULL) {
-            f = new(ctx) ir_function(name);
-            state->symbols->add_global_function(f);
-            emit_function(state, f);
-         }
-         sig = sig->clone_prototype(f, NULL);
-         f->add_signature(sig);
-      }
-   }
     return sig;
  }

@@ -2142,6 +2130,16 @@ ast_function_expression::hir(exec_list *instructions,
                                           this->expressions)) {
           /* an error has already been emitted */
           value = ir_rvalue::error_value(ctx);
+      } else if (sig->is_builtin() && strcmp(func_name, "ftransform") == 0) {
+         /* ftransform refers to global variables, and we don't have any code
+          * for remapping the variable references in the built-in shader.
+          */
+         ir_variable *mvp =
+            state->symbols->get_variable("gl_ModelViewProjectionMatrix");
+         ir_variable *vtx = state->symbols->get_variable("gl_Vertex");
+         value = new(ctx) ir_expression(ir_binop_mul, glsl_type::vec4_type,
+                                        new(ctx) ir_dereference_variable(mvp),
+                                        new(ctx) ir_dereference_variable(vtx));
        } else {
           if (state->stage == MESA_SHADER_TESS_CTRL &&
               sig->is_builtin() && strcmp(func_name, "barrier") == 0) {
@@ -2162,8 +2160,8 @@ ast_function_expression::hir(exec_list *instructions,
              }
           }

-         value = generate_call(instructions, sig,
-                               &actual_parameters, sub_var, array_idx, state);
+         value = generate_call(instructions, sig, &actual_parameters, sub_var,
+                               array_idx, state, sig->is_builtin());
           if (!value) {
              ir_variable *const tmp = new(ctx) 
ir_variable(glsl_type::void_type,
                                                            "void_var",

_______________________________________________
mesa-commit mailing list
mesa-com...@lists.freedesktop.org
https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.freedesktop.org_mailman_listinfo_mesa-2Dcommit&d=CwIGaQ&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=T0t4QG7chq2ZwJo6wilkFznRSFy-8uDKartPGbomVj8&m=m7lwXZjH2_UAMD5u1FWrl6EmaAyly794Od4UBt09XC4&s=t4yJluombqrkqOp-ONykE-g6aa-PtPdZ38OSClJnoyc&e=


[require]
GLSL >= 1.20

[vertex shader]

invariant gl_Position;

void main(void)
{
   gl_Position = ftransform();
}



[fragment shader]

void main(void)
{
   gl_FragColor = vec4(1.0);
}


[test]

_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Reply via email to