headers
Ian Romanick | 1 Oct 2011 01:24

[PATCH] i965: Fix swizzle related assertion

From: Ian Romanick <ian.d.romanick <at> intel.com>

As innocuous as it seemed, ebca47a basically broke the world (e.g.,
>200 piglit regressions).  In vec4_visitor::emit_block_move,
src->swizzle was expected to be BRW_SWIZZLE_NOOP before setting it to
a swizzle that would replicate the existing channels of the source
type to a vec4 (e.g., .xyyy for a vec2).

The original assertion seems to have been a little bogus.  In addition
to being BRW_SWIZZLE_NOOP, src->swizzle might already be a swizzle
that would replicate the existing channels of the source type to a
vec4.  In other words, it might already have the value that we're
about to assign to it.

Signed-off-by: Ian Romanick <ian.d.romanick <at> intel.com>
---
 src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp b/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp
index 5815e31..9420684 100644
--- a/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp
+++ b/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp
 <at>  <at>  -1472,7 +1472,8  <at>  <at>  vec4_visitor::emit_block_move(dst_reg *dst, src_reg *src,
    dst->writemask = (1 << type->vector_elements) - 1;

    /* Do we need to worry about swizzling a swizzle? */
-   assert(src->swizzle == BRW_SWIZZLE_NOOP);
+   assert(src->swizzle == BRW_SWIZZLE_NOOP
+	  || src->swizzle == swizzle_for_size(type->vector_elements));
(Continue reading)

Permalink | Reply | 0 comments |
headers
Ian Romanick | 1 Oct 2011 01:44

[PATCH 00/13] Clean up shader attribute handling v2

A couple of the patches from the original series have already been
pushed.  A bunch of the others earned some feedback, and these have
been reworked.  The primary changes in v2 are:

 - Fix some bad squash-merging related to patch 2.

 - Delete the post-link symbol table because it is full of lies (patch
   01/13).

 - Add and use string_to_uint_map (patch 04/13, 06/13, and 07/13)
   instead of the crazy casting of hash_table data pointers to / from
   intptr_t.

The patches that already have Ken's Reviewed-by listed are unchanged
from v1.

 src/glsl/linker.cpp                        |  146 +++++++++---------
 src/mesa/main/mtypes.h                     |   13 +-
 src/mesa/main/shader_query.cpp             |  239 ++++++++++++++++++++++++++++
 src/mesa/main/shaderapi.c                  |  145 +-----------------
 src/mesa/main/shaderapi.h                  |    4 +
 src/mesa/main/shaderobj.c                  |   11 +-
 src/mesa/program/hash_table.c              |   25 +++
 src/mesa/program/hash_table.h              |   94 +++++++++++-
 src/mesa/program/ir_to_mesa.cpp            |    9 -
 src/mesa/program/prog_parameter.c          |   22 ---
 src/mesa/program/prog_parameter.h          |    4 -
 src/mesa/program/program.c                 |    5 -
 src/mesa/program/string_to_uint_map.cpp    |   42 +++++
 src/mesa/sources.mak                       |    6 +-
(Continue reading)

Permalink | Reply | 37 comments |
headers
Ian Romanick | 1 Oct 2011 01:44

[PATCH 01/13] glsl: Delete symbol table in post-link shaders

From: Ian Romanick <ian.d.romanick <at> intel.com>

The symbol table in the linked shaders may contain references to
variables that were removed (e.g., unused uniforms).  Since it may
contain junk, there is no possible valid use.  Delete it and set the
pointer to NULL.

Signed-off-by: Ian Romanick <ian.d.romanick <at> intel.com>
---
 src/glsl/linker.cpp |    8 ++++++++
 1 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/src/glsl/linker.cpp b/src/glsl/linker.cpp
index d802a0a..6d40dd2 100644
--- a/src/glsl/linker.cpp
+++ b/src/glsl/linker.cpp
 <at>  <at>  -1831,6 +1831,14  <at>  <at>  done:

       /* Retain any live IR, but trash the rest. */
       reparent_ir(prog->_LinkedShaders[i]->ir, prog->_LinkedShaders[i]->ir);
+
+      /* The symbol table in the linked shaders may contain references to
+       * variables that were removed (e.g., unused uniforms).  Since it may
+       * contain junk, there is no possible valid use.  Delete it and set the
+       * pointer to NULL.
+       */
+      delete prog->_LinkedShaders[i]->symbols;
+      prog->_LinkedShaders[i]->symbols = NULL;
    }
(Continue reading)

Permalink | Reply | 0 comments |
headers
Ian Romanick | 1 Oct 2011 01:44

[PATCH 02/13] mesa: Move _mesa_GetAttribLocationARB to shader_query.cpp

From: Ian Romanick <ian.d.romanick <at> intel.com>

This allows querying the linked shader itself rather than the Mesa IR.
This is the first step towards removing gl_program::Attributes.

Signed-off-by: Ian Romanick <ian.d.romanick <at> intel.com>
---
 src/mesa/main/shader_query.cpp |   88 ++++++++++++++++++++++++++++++++++++++++
 src/mesa/main/shaderapi.c      |   41 ------------------
 src/mesa/sources.mak           |    3 +-
 3 files changed, 90 insertions(+), 42 deletions(-)
 create mode 100644 src/mesa/main/shader_query.cpp

diff --git a/src/mesa/main/shader_query.cpp b/src/mesa/main/shader_query.cpp
new file mode 100644
index 0000000..d5c1546
--- /dev/null
+++ b/src/mesa/main/shader_query.cpp
 <at>  <at>  -0,0 +1,88  <at>  <at> 
+/*
+ * Copyright © 2011 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
(Continue reading)

Permalink | Reply | 0 comments |
headers
Ian Romanick | 1 Oct 2011 01:44

[PATCH 03/13] mesa: Add hash_table_replace

From: Ian Romanick <ian.d.romanick <at> intel.com>

hash_table_replace doesn't use get_node to avoid having to hash the key twice.

Signed-off-by: Ian Romanick <ian.d.romanick <at> intel.com>
Reviewed-by: Kenneth Graunke <kenneth <at> whitecape.org>
---
 src/mesa/program/hash_table.c |   25 +++++++++++++++++++++++++
 src/mesa/program/hash_table.h |   15 +++++++++++++++
 2 files changed, 40 insertions(+), 0 deletions(-)

diff --git a/src/mesa/program/hash_table.c b/src/mesa/program/hash_table.c
index 2b09462..dc8563a 100644
--- a/src/mesa/program/hash_table.c
+++ b/src/mesa/program/hash_table.c
 <at>  <at>  -150,6 +150,31  <at>  <at>  hash_table_insert(struct hash_table *ht, void *data, const void *key)
 }

 void
+hash_table_replace(struct hash_table *ht, void *data, const void *key)
+{
+    const unsigned hash_value = (*ht->hash)(key);
+    const unsigned bucket = hash_value % ht->num_buckets;
+    struct node *node;
+    struct hash_node *hn;
+
+    foreach(node, & ht->buckets[bucket]) {
+       hn = (struct hash_node *) node;
+
+       if ((*ht->compare)(hn->key, key) == 0) {
(Continue reading)

Permalink | Reply | 0 comments |
headers
Ian Romanick | 1 Oct 2011 01:44

[PATCH 05/13] mesa: Move _mesa_BindAttribLocationARB to shader_query.cpp

From: Ian Romanick <ian.d.romanick <at> intel.com>

This just folds bind_attrib_location into _mesa_BindAttribLocationARB
and moves the resulting function function to the other source file.
More changes are coming soon.

Signed-off-by: Ian Romanick <ian.d.romanick <at> intel.com>
Reviewed-by: Kenneth Graunke <kenneth <at> whitecape.org>
---
 src/mesa/main/shader_query.cpp |   42 ++++++++++++++++++++++++++++++++
 src/mesa/main/shaderapi.c      |   52 ----------------------------------------
 2 files changed, 42 insertions(+), 52 deletions(-)

diff --git a/src/mesa/main/shader_query.cpp b/src/mesa/main/shader_query.cpp
index d5c1546..b754b1f 100644
--- a/src/mesa/main/shader_query.cpp
+++ b/src/mesa/main/shader_query.cpp
 <at>  <at>  -37,6 +37,48  <at>  <at>  extern "C" {
 #include "shaderapi.h"
 }

+void GLAPIENTRY
+_mesa_BindAttribLocationARB(GLhandleARB program, GLuint index,
+                            const GLcharARB *name)
+{
+   GET_CURRENT_CONTEXT(ctx);
+
+   const GLint size = -1; /* unknown size */
+   GLint i;
+   GLenum datatype = GL_FLOAT_VEC4;
(Continue reading)

Permalink | Reply | 0 comments |
headers
Ian Romanick | 1 Oct 2011 01:44

[PATCH 04/13] mesa: Add string_to_uint_map facade class

From: Ian Romanick <ian.d.romanick <at> intel.com>

Signed-off-by: Ian Romanick <ian.d.romanick <at> intel.com>
---
 src/mesa/program/hash_table.h           |   79 ++++++++++++++++++++++++++++++-
 src/mesa/program/string_to_uint_map.cpp |   42 ++++++++++++++++
 src/mesa/sources.mak                    |    3 +-
 3 files changed, 122 insertions(+), 2 deletions(-)
 create mode 100644 src/mesa/program/string_to_uint_map.cpp

diff --git a/src/mesa/program/hash_table.h b/src/mesa/program/hash_table.h
index e7ab067..bfe221b 100644
--- a/src/mesa/program/hash_table.h
+++ b/src/mesa/program/hash_table.h
 <at>  <at>  -31,7 +31,13  <at>  <at> 
 #ifndef HASH_TABLE_H
 #define HASH_TABLE_H

+#include <string.h>
+#include <stdint.h>
+#include <limits.h>
+#include <assert.h>
+
 struct hash_table;
+struct string_to_uint_map;

 typedef unsigned (*hash_func_t)(const void *key);
 typedef int (*hash_compare_func_t)(const void *key1, const void *key2);
 <at>  <at>  -171,7 +177,78  <at>  <at>  hash_table_call_foreach(struct hash_table *ht,
 					 void *closure),
(Continue reading)

Permalink | Reply | 0 comments |
headers
Ian Romanick | 1 Oct 2011 01:44

[PATCH 06/13] mesa: Add gl_shader_program::AttributeBindings

From: Ian Romanick <ian.d.romanick <at> intel.com>

This currently mirrors the state tracking
gl_shader_program::Attributes, but I'm working towards eliminating
that.

Signed-off-by: Ian Romanick <ian.d.romanick <at> intel.com>
---
 src/mesa/main/mtypes.h         |   11 ++++++++++-
 src/mesa/main/shader_query.cpp |    5 +++++
 src/mesa/main/shaderobj.c      |    9 +++++++++
 3 files changed, 24 insertions(+), 1 deletions(-)

diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h
index 70f33ff..b9296a8 100644
--- a/src/mesa/main/mtypes.h
+++ b/src/mesa/main/mtypes.h
 <at>  <at>  -2146,8 +2146,17  <at>  <at>  struct gl_shader_program
    GLuint NumShaders;          /**≤ number of attached shaders */
    struct gl_shader **Shaders; /**≤ List of attached the shaders */

-   /** User-defined attribute bindings (glBindAttribLocation) */
+   /**
+    * User-defined attribute bindings
+    *
+    * These are set via \c glBindAttribLocation and are used to direct the
+    * GLSL linker.  These are \b not the values used in the compiled shader,
+    * and they are \b not the values returned by \c glGetAttribLocation.
+    *
+    * \sa gl_program::Attributes
(Continue reading)

Permalink | Reply | 0 comments |
headers
Ian Romanick | 1 Oct 2011 01:44

[PATCH 07/13] linker: Use gl_shader_program::AttributeBindings for attrib locations

From: Ian Romanick <ian.d.romanick <at> intel.com>

Signed-off-by: Ian Romanick <ian.d.romanick <at> intel.com>
---
 src/glsl/linker.cpp |  138 +++++++++++++++++++++++---------------------------
 1 files changed, 64 insertions(+), 74 deletions(-)

diff --git a/src/glsl/linker.cpp b/src/glsl/linker.cpp
index 6d40dd2..9463f53 100644
--- a/src/glsl/linker.cpp
+++ b/src/glsl/linker.cpp
 <at>  <at>  -1298,71 +1298,6  <at>  <at>  assign_attribute_or_color_locations(gl_shader_program *prog,

    invalidate_variable_locations(sh, direction, generic_base);

-   if ((target_index == MESA_SHADER_VERTEX) && (prog->Attributes != NULL)) {
-      for (unsigned i = 0; i < prog->Attributes->NumParameters; i++) {
-	 ir_variable *const var =
-	    sh->symbols->get_variable(prog->Attributes->Parameters[i].Name);
-
-	 /* Note: attributes that occupy multiple slots, such as arrays or
-	  * matrices, may appear in the attrib array multiple times.
-	  */
-	 if ((var == NULL) || (var->location != -1))
-	    continue;
-
-	 /* From page 61 of the OpenGL 4.0 spec:
-	  *
-	  *     "LinkProgram will fail if the attribute bindings assigned by
-	  *     BindAttribLocation do not leave not enough space to assign a
(Continue reading)

Permalink | Reply | 6 comments |
headers
Ian Romanick | 1 Oct 2011 01:44

[PATCH 08/13] mesa: Move _mesa_GetActiveAttribARB to shader_query.cpp

From: Ian Romanick <ian.d.romanick <at> intel.com>

This just folds get_active_attrib into _mesa_GetActiveAttribARB
and moves the resulting function function to the other source file.
More changes are coming soon.

Signed-off-by: Ian Romanick <ian.d.romanick <at> intel.com>
Reviewed-by: Kenneth Graunke <kenneth <at> whitecape.org>
---
 src/mesa/main/shader_query.cpp |   32 ++++++++++++++++++++++++++++++
 src/mesa/main/shaderapi.c      |   42 ----------------------------------------
 2 files changed, 32 insertions(+), 42 deletions(-)

diff --git a/src/mesa/main/shader_query.cpp b/src/mesa/main/shader_query.cpp
index 424610d..6e81564 100644
--- a/src/mesa/main/shader_query.cpp
+++ b/src/mesa/main/shader_query.cpp
 <at>  <at>  -84,6 +84,38  <at>  <at>  _mesa_BindAttribLocationARB(GLhandleARB program, GLuint index,
     */
 }

+void GLAPIENTRY
+_mesa_GetActiveAttribARB(GLhandleARB program, GLuint index,
+                         GLsizei maxLength, GLsizei * length, GLint * size,
+                         GLenum * type, GLcharARB * name)
+{
+   GET_CURRENT_CONTEXT(ctx);
+   const struct gl_program_parameter_list *attribs = NULL;
+   struct gl_shader_program *shProg;
+
(Continue reading)

Permalink | Reply | 0 comments |

Gmane