Re: [Mesa-dev] [PATCH 2/7] glsl: add support for ARB_blend_func_extended (v2)

2012-04-10 Thread Dave Airlie
On Mon, Apr 9, 2012 at 9:13 PM, Eric Anholt e...@anholt.net wrote:
 On Tue,  3 Apr 2012 14:16:52 +0100, Dave Airlie airl...@gmail.com wrote:
 From: Dave Airlie airl...@redhat.com

 This adds index support to the GLSL compiler.

 I'm not 100% sure of my approach here, esp without how output ordering
 happens wrt location, index pairs, in the mark function.

 Since current hw doesn't ever have a location  0 with an index  0,
 we don't have to work out if the output ordering the hw requires is
 location, index, location, index or location, location, index, index.
 But we have no hw to know, so punt on it for now.

 v2: index requires layout - catch and error
     setup explicit index properly.

 I don't like this magic 4 that's unexplained.  It seems like the second
 fragment output should have a totally separate ir_variable-location
 assigned.  Whether that's in a separate space like FRAG_RESULT_DUALSRC_0
 that comes after all the FRAG_RESULT_DATAn, or whether it's just
 FRAG_RESULT_DATA1 since we're not anticipating dual source max buffers
 going beyond 1, 4 doesn't seem like the right number.

Yeah the reason I left it all configurable was I wasn't sure how
flexible we should leave the GLSL compiler.

But I personally don't see hw with index  1 and location  0 unless
index gets re-purposed for something else.

I'll resend a smaller patch that just puts index after location.

Dave.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 2/7] glsl: add support for ARB_blend_func_extended (v2)

2012-04-09 Thread Eric Anholt
On Tue,  3 Apr 2012 14:16:52 +0100, Dave Airlie airl...@gmail.com wrote:
 From: Dave Airlie airl...@redhat.com
 
 This adds index support to the GLSL compiler.
 
 I'm not 100% sure of my approach here, esp without how output ordering
 happens wrt location, index pairs, in the mark function.
 
 Since current hw doesn't ever have a location  0 with an index  0,
 we don't have to work out if the output ordering the hw requires is
 location, index, location, index or location, location, index, index.
 But we have no hw to know, so punt on it for now.
 
 v2: index requires layout - catch and error
 setup explicit index properly.

I don't like this magic 4 that's unexplained.  It seems like the second
fragment output should have a totally separate ir_variable-location
assigned.  Whether that's in a separate space like FRAG_RESULT_DUALSRC_0
that comes after all the FRAG_RESULT_DATAn, or whether it's just
FRAG_RESULT_DATA1 since we're not anticipating dual source max buffers
going beyond 1, 4 doesn't seem like the right number.


pgpjbqfz4GNeN.pgp
Description: PGP signature
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] [PATCH 2/7] glsl: add support for ARB_blend_func_extended (v2)

2012-04-03 Thread Dave Airlie
From: Dave Airlie airl...@redhat.com

This adds index support to the GLSL compiler.

I'm not 100% sure of my approach here, esp without how output ordering
happens wrt location, index pairs, in the mark function.

Since current hw doesn't ever have a location  0 with an index  0,
we don't have to work out if the output ordering the hw requires is
location, index, location, index or location, location, index, index.
But we have no hw to know, so punt on it for now.

v2: index requires layout - catch and error
setup explicit index properly.

Signed-off-by: Dave Airlie airl...@redhat.com
---
 src/glsl/ast.h |   12 
 src/glsl/ast_to_hir.cpp|9 -
 src/glsl/builtin_variables.cpp |1 +
 src/glsl/glsl_parser.yy|   20 
 src/glsl/ir.h  |1 +
 src/glsl/ir_clone.cpp  |4 
 src/glsl/ir_set_program_inouts.cpp |   12 ++--
 src/glsl/linker.cpp|   13 +
 8 files changed, 61 insertions(+), 11 deletions(-)

diff --git a/src/glsl/ast.h b/src/glsl/ast.h
index 1f78af8..9a7b2a2 100644
--- a/src/glsl/ast.h
+++ b/src/glsl/ast.h
@@ -363,6 +363,11 @@ struct ast_type_qualifier {
  * qualifier is used.
  */
 unsigned explicit_location:1;
+/**
+ * Flag set if GL_ARB_explicit_attrib_location index layout
+ * qualifier is used.
+ */
+unsigned explicit_index:1;
 
  /** \name Layout qualifiers for GL_AMD_conservative_depth */
  /** \{ */
@@ -386,6 +391,13 @@ struct ast_type_qualifier {
 * This field is only valid if \c explicit_location is set.
 */
int location;
+   /**
+* Index specified via GL_ARB_explicit_attrib_location layout
+*
+* \note
+* This field is only valid if \c explicit_index is set.
+*/
+   int index;
 
/**
 * Return true if and only if an interpolation qualifier is present.
diff --git a/src/glsl/ast_to_hir.cpp b/src/glsl/ast_to_hir.cpp
index 6210eb1..2841469 100644
--- a/src/glsl/ast_to_hir.cpp
+++ b/src/glsl/ast_to_hir.cpp
@@ -2092,14 +2092,21 @@ apply_type_qualifier_to_variable(const struct 
ast_type_qualifier *qual,
 } else {
var-location = qual-location;
 }
+if (qual-flags.q.explicit_index) {
+   var-explicit_index = true;
+   var-index = qual-index;
+}
   }
+   } else if (qual-flags.q.explicit_index) {
+_mesa_glsl_error(loc, state,
+ explicit index requires explicit location\n);
}
 
/* Does the declaration use the 'layout' keyword?
 */
const bool uses_layout = qual-flags.q.pixel_center_integer
   || qual-flags.q.origin_upper_left
-  || qual-flags.q.explicit_location;
+  || qual-flags.q.explicit_location; /* no need for index since it relies 
on location */
 
/* Does the declaration use the deprecated 'attribute' or 'varying'
 * keywords?
diff --git a/src/glsl/builtin_variables.cpp b/src/glsl/builtin_variables.cpp
index 516a69c..03b64c9 100644
--- a/src/glsl/builtin_variables.cpp
+++ b/src/glsl/builtin_variables.cpp
@@ -408,6 +408,7 @@ add_variable(exec_list *instructions, glsl_symbol_table 
*symtab,
 
var-location = slot;
var-explicit_location = (slot = 0);
+   var-explicit_index = 0;
 
/* Once the variable is created an initialized, add it to the symbol table
 * and add the declaration to the IR stream.
diff --git a/src/glsl/glsl_parser.yy b/src/glsl/glsl_parser.yy
index 64506b6..e45089a 100644
--- a/src/glsl/glsl_parser.yy
+++ b/src/glsl/glsl_parser.yy
@@ -1103,8 +1103,14 @@ layout_qualifier_id_list:
   if ($1.flags.q.explicit_location)
  $$.location = $1.location;
 
+  if ($1.flags.q.explicit_index)
+ $$.index = $1.index;
+
   if ($3.flags.q.explicit_location)
  $$.location = $3.location;
+
+  if ($3.flags.q.explicit_index)
+ $$.index = $3.index;
}
;
 
@@ -1191,6 +1197,20 @@ layout_qualifier_id:
YYERROR;
 }
  }
+
+ if (strcmp(index, $1) == 0) {
+got_one = true;
+
+$$.flags.q.explicit_index = 1;
+
+if ($3 = 0) {
+   $$.index = $3;
+} else {
+   _mesa_glsl_error( @3, state,
+invalid index %d specified\n, $3);
+YYERROR;
+ }
+  }
   }
 
   /* If the identifier didn't match any known layout identifiers,
diff --git a/src/glsl/ir.h b/src/glsl/ir.h
index 9450e8f..d6c6a60 100644
--- a/src/glsl/ir.h
+++ b/src/glsl/ir.h
@@ -386,6 +386,7 @@ public:
 * no effect).
 */
unsigned explicit_location:1;
+   unsigned explicit_index:1;
 
/**
 * Does this variable have an initializer?
diff --git a/src/glsl/ir_clone.cpp