Re: [Mesa-dev] [PATCH 5/5] glsl: Clarify ir_function::matching_sigature()

2011-08-01 Thread Chris Bandy
On 07/29/2011 08:59 PM, Chad Versace wrote:
 The function used a variable named 'score', which was an outright lie.
 A signature matches or it doesn't; there is no fuzzy scoring.

 Change the return type of parameter_lists_match() to an enum, and
 let ir_function::matching_sigature() switch on that enum.

 CC: Ian Romanick ian.d.roman...@intel.com
 CC: Kenneth Graunke kenn...@whitecape.org
 Signed-off-by: Chad Versace c...@chad-versace.us
 ---
  src/glsl/ir_function.cpp |   53 -
  1 files changed, 33 insertions(+), 20 deletions(-)

 diff --git a/src/glsl/ir_function.cpp b/src/glsl/ir_function.cpp
 index dd63e30..6cfc32c 100644
 --- a/src/glsl/ir_function.cpp
 +++ b/src/glsl/ir_function.cpp
 @@ -24,17 +24,28 @@
  #include glsl_types.h
  #include ir.h
  
 +typedef enum {
 +   PARAMETER_LIST_NO_MATCH,
 +   PARAMETER_LIST_EXACT_MATCH,
 +   PARAMETER_LIST_INEXACT_MATCH, /* Match requires implicit conversion. */
 +} parameter_list_match_t;
 +
  /**
   * \brief Check if two parameter lists match.
   *
   * \param list_a Parameters of the function definition.
   * \param list_b Actual parameters passed to the function.
 - * \return If an exact match, return 0.
 - * If an inexact match requiring implicit conversion, return 1.
 - * If not a match, return -1.
   * \see matching_signature()
   */

Looks like the above comment block is now duplicated here.

 -static int
 +
 +/**
 + * \brief Check if two parameter lists match.
 + *
 + * \param list_a Parameters of the function definition.
 + * \param list_b Actual parameters passed to the function.
 + * \see matching_signature()
 + */
 +static parameter_list_match_t
  parameter_lists_match(const exec_list *list_a, const exec_list *list_b)
  {
 const exec_node *node_a = list_a-head;
 @@ -52,7 +63,7 @@ parameter_lists_match(const exec_list *list_a, const 
 exec_list *list_b)
 * do not match.
 */
if (node_b-is_tail_sentinel())
 -  return -1;
 +  return PARAMETER_LIST_NO_MATCH;
  
  
const ir_variable *const param = (ir_variable *) node_a;
 @@ -72,17 +83,17 @@ parameter_lists_match(const exec_list *list_a, const 
 exec_list *list_b)
 * as uniform.
 */
assert(0);
 -  return -1;
 +  return PARAMETER_LIST_NO_MATCH;
  
case ir_var_const_in:
case ir_var_in:
if (!actual-type-can_implicitly_convert_to(param-type))
 - return -1;
 + return PARAMETER_LIST_NO_MATCH;
break;
  
case ir_var_out:
if (!param-type-can_implicitly_convert_to(actual-type))
 - return -1;
 + return PARAMETER_LIST_NO_MATCH;
break;
  
case ir_var_inout:
 @@ -90,11 +101,11 @@ parameter_lists_match(const exec_list *list_a, const 
 exec_list *list_b)
 * there is int - float but no float - int), inout parameters must
 * be exact matches.
 */
 -  return -1;
 +  return PARAMETER_LIST_NO_MATCH;
  
default:
assert(false);
 -  return -1;
 +  return PARAMETER_LIST_NO_MATCH;
}
 }
  
 @@ -103,12 +114,12 @@ parameter_lists_match(const exec_list *list_a, const 
 exec_list *list_b)
  * match.
  */
 if (!node_b-is_tail_sentinel())
 -  return -1;
 +  return PARAMETER_LIST_NO_MATCH;
  
 if (inexact_match)
 -  return 1;
 +  return PARAMETER_LIST_INEXACT_MATCH;
 else
 -  return 0;
 +  return PARAMETER_LIST_EXACT_MATCH;
  }
  
  
 @@ -132,18 +143,20 @@ ir_function::matching_signature(const exec_list 
 *actual_parameters)
ir_function_signature *const sig =
(ir_function_signature *) iter.get();
  
 -  const int score = parameter_lists_match( sig-parameters,
 -   actual_parameters);
 -
 -  /* If we found an exact match, simply return it */
 -  if (score == 0)
 +  switch (parameter_lists_match( sig-parameters, actual_parameters)) {
 +  case PARAMETER_LIST_EXACT_MATCH:
return sig;
 -
 -  if (score  0) {
 +  case PARAMETER_LIST_INEXACT_MATCH:
if (match == NULL)
   match = sig;
else
   multiple_inexact_matches = true;
 +  continue;
 +  case PARAMETER_LIST_NO_MATCH:
 +  continue;
 +  default:
 +  assert(false);
 +  return NULL;
}
 }
  

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


[Mesa-dev] [Bug 3165] texImage.IsCompressed and texImage.CompressedSize issues

2011-08-01 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=3165

--- Comment #8 from Ian Romanick i...@freedesktop.org 2011-08-01 09:18:59 PDT 
---
If I'm not mistaken, most of the issues from this bug have been fixed for some
time.  The one remaining issue was returning the wrong format for
GL_TEXTURE_INTERNAL_FORMAT queries.  That should be fixed by the following
series on Mesa master.  If that's the case, I'll pick those patches to the 7.11
and 7.10 branches and close this old, old bug.

commit b189d1635d89cd7d900e8f9a5eed88d7dc0b46cb
Author: Ian Romanick ian.d.roman...@intel.com
Date:   Fri Jul 22 16:45:50 2011 -0700

mesa: Make _mesa_get_compressed_formats match the texture compression specs

The implementation deviated slightly from the GL_EXT_texture_sRGB spec
and from other implementations.  A giant comment block was added to
justify the somewhat odd behavior of this function.

In addition, the interface had unnecessary cruft.  The 'all' parameter
was false at all callers, so it has been removed.

Reviewed-by: Brian Paul bri...@vmware.com

commit 143b65f7612c255f29d08392192098b1c2bf4b62
Author: Ian Romanick ian.d.roman...@intel.com
Date:   Fri Jul 22 15:26:24 2011 -0700

mesa: Return the correct internal fmt when a generic compressed fmt was
used

If an application requests a generic compressed format for a texture
and the driver does not pick a specific compressed format, return the
generic base format (e.g., GL_RGBA) for the GL_TEXTURE_INTERNAL_FORMAT
query.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=3165
Reviewed-by: Brian Paul bri...@vmware.com

commit 09916e877fc14723d7950f892e181df9f7d7f36f
Author: Ian Romanick ian.d.roman...@intel.com
Date:   Fri Jul 22 15:25:55 2011 -0700

mesa: Add utility function to get base format from a GL compressed format

Reviewed-by: Brian Paul bri...@vmware.com

-- 
Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are the assignee for the bug.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 5/5] glsl: Clarify ir_function::matching_sigature()

2011-08-01 Thread Chad Versace
On 08/01/2011 08:05 AM, Chris Bandy wrote:
 On 07/29/2011 08:59 PM, Chad Versace wrote:
 The function used a variable named 'score', which was an outright lie.
 A signature matches or it doesn't; there is no fuzzy scoring.

 Change the return type of parameter_lists_match() to an enum, and
 let ir_function::matching_sigature() switch on that enum.

 CC: Ian Romanick ian.d.roman...@intel.com
 CC: Kenneth Graunke kenn...@whitecape.org
 Signed-off-by: Chad Versace c...@chad-versace.us
 ---
  src/glsl/ir_function.cpp |   53 
 -
  1 files changed, 33 insertions(+), 20 deletions(-)

 diff --git a/src/glsl/ir_function.cpp b/src/glsl/ir_function.cpp
 index dd63e30..6cfc32c 100644
 --- a/src/glsl/ir_function.cpp
 +++ b/src/glsl/ir_function.cpp
 @@ -24,17 +24,28 @@
  #include glsl_types.h
  #include ir.h
  
 +typedef enum {
 +   PARAMETER_LIST_NO_MATCH,
 +   PARAMETER_LIST_EXACT_MATCH,
 +   PARAMETER_LIST_INEXACT_MATCH, /* Match requires implicit conversion. */
 +} parameter_list_match_t;
 +
  /**
   * \brief Check if two parameter lists match.
   *
   * \param list_a Parameters of the function definition.
   * \param list_b Actual parameters passed to the function.
 - * \return If an exact match, return 0.
 - * If an inexact match requiring implicit conversion, return 1.
 - * If not a match, return -1.
   * \see matching_signature()
   */
 
 Looks like the above comment block is now duplicated here.

Thanks Chris.

I just pushed the fix.

commit 5541920e0ac4ea8383c7f896daba24a304aafec6
Author: Chad Versace c...@chad-versace.us
Date:   Mon Aug 1 09:36:08 2011 -0700

glsl: Remove duplicate comment

Remove duplicate doxgen comment for
ir_function.cpp:parameter_lists_match().

Signed-off-by: Chad Versace c...@chad-versace.us

-- 
Chad Versace
c...@chad-versace.us
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] [PATCH 1/7] linker: Make linker_error_printf set LinkStatus to false

2011-08-01 Thread Ian Romanick
From: Ian Romanick ian.d.roman...@intel.com

Remove the other places that set LinkStatus to false since they all
immediately follow a call to linker_error_printf.
---
 src/glsl/linker.cpp |8 ++--
 1 files changed, 2 insertions(+), 6 deletions(-)

diff --git a/src/glsl/linker.cpp b/src/glsl/linker.cpp
index fe570b6..c260f29 100644
--- a/src/glsl/linker.cpp
+++ b/src/glsl/linker.cpp
@@ -172,6 +172,8 @@ linker_error_printf(gl_shader_program *prog, const char 
*fmt, ...)
va_start(ap, fmt);
ralloc_vasprintf_append(prog-InfoLog, fmt, ap);
va_end(ap);
+
+   prog-LinkStatus = false;
 }
 
 
@@ -1527,7 +1529,6 @@ assign_varying_locations(struct gl_context *ctx,
 
linker_error_printf(prog, fragment shader varying %s not written 
by vertex shader\n., var-name);
-   prog-LinkStatus = false;
 }
 
 /* An 'in' variable is only really a shader input if its
@@ -1720,12 +1721,10 @@ link_shaders(struct gl_context *ctx, struct 
gl_shader_program *prog)
 * FINISHME: at least 16, so hardcode 16 for now.
 */
if (!assign_attribute_or_color_locations(prog, MESA_SHADER_VERTEX, 16)) {
-  prog-LinkStatus = false;
   goto done;
}
 
if (!assign_attribute_or_color_locations(prog, MESA_SHADER_FRAGMENT, 
ctx-Const.MaxDrawBuffers)) {
-  prog-LinkStatus = false;
   goto done;
}
 
@@ -1742,7 +1741,6 @@ link_shaders(struct gl_context *ctx, struct 
gl_shader_program *prog)
   if (!assign_varying_locations(ctx, prog,
prog-_LinkedShaders[prev],
prog-_LinkedShaders[i])) {
-prog-LinkStatus = false;
 goto done;
   }
 
@@ -1775,10 +1773,8 @@ link_shaders(struct gl_context *ctx, struct 
gl_shader_program *prog)
if (ctx-API == API_OPENGLES2 || prog-Version == 100) {
   if (prog-_LinkedShaders[MESA_SHADER_VERTEX] == NULL) {
 linker_error_printf(prog, program lacks a vertex shader\n);
-prog-LinkStatus = false;
   } else if (prog-_LinkedShaders[MESA_SHADER_FRAGMENT] == NULL) {
 linker_error_printf(prog, program lacks a fragment shader\n);
-prog-LinkStatus = false;
   }
}
 
-- 
1.7.4.4

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


[Mesa-dev] [PATCH 2/7] linker: Make linker_{error, warning}_printf generally available

2011-08-01 Thread Ian Romanick
From: Ian Romanick ian.d.roman...@intel.com

linker_warning_printf is a new function.  It's identical to
linker_error_printf except that it doesn't set LinkStatus=false and it
prepends warning:  on messages instead of error: .
---
 src/glsl/ir_function_detect_recursion.cpp |1 +
 src/glsl/linker.cpp   |   13 +
 src/glsl/linker.h |3 ---
 src/glsl/program.h|8 
 4 files changed, 22 insertions(+), 3 deletions(-)

diff --git a/src/glsl/ir_function_detect_recursion.cpp 
b/src/glsl/ir_function_detect_recursion.cpp
index 44a1cd0..e79f029 100644
--- a/src/glsl/ir_function_detect_recursion.cpp
+++ b/src/glsl/ir_function_detect_recursion.cpp
@@ -125,6 +125,7 @@
 #include glsl_parser_extras.h
 #include linker.h
 #include program/hash_table.h
+#include program.h
 
 struct call_node : public exec_node {
class function *func;
diff --git a/src/glsl/linker.cpp b/src/glsl/linker.cpp
index c260f29..994c631 100644
--- a/src/glsl/linker.cpp
+++ b/src/glsl/linker.cpp
@@ -178,6 +178,19 @@ linker_error_printf(gl_shader_program *prog, const char 
*fmt, ...)
 
 
 void
+linker_warning_printf(gl_shader_program *prog, const char *fmt, ...)
+{
+   va_list ap;
+
+   ralloc_strcat(prog-InfoLog, error: );
+   va_start(ap, fmt);
+   ralloc_vasprintf_append(prog-InfoLog, fmt, ap);
+   va_end(ap);
+
+}
+
+
+void
 invalidate_variable_locations(gl_shader *sh, enum ir_variable_mode mode,
  int generic_base)
 {
diff --git a/src/glsl/linker.h b/src/glsl/linker.h
index a8ce16a..769cf68 100644
--- a/src/glsl/linker.h
+++ b/src/glsl/linker.h
@@ -25,9 +25,6 @@
 #ifndef GLSL_LINKER_H
 #define GLSL_LINKER_H
 
-extern void
-linker_error_printf(gl_shader_program *prog, const char *fmt, ...);
-
 extern bool
 link_function_calls(gl_shader_program *prog, gl_shader *main,
gl_shader **shader_list, unsigned num_shaders);
diff --git a/src/glsl/program.h b/src/glsl/program.h
index db602fa..2f147bc 100644
--- a/src/glsl/program.h
+++ b/src/glsl/program.h
@@ -25,3 +25,11 @@
 
 extern void
 link_shaders(struct gl_context *ctx, struct gl_shader_program *prog);
+
+extern void
+linker_error_printf(gl_shader_program *prog, const char *fmt, ...)
+   PRINTFLIKE(2, 3);
+
+extern void
+linker_warning_printf(gl_shader_program *prog, const char *fmt, ...)
+   PRINTFLIKE(2, 3);
-- 
1.7.4.4

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


[Mesa-dev] [PATCH 3/7] mesa: Ensure that gl_shader_program::InfoLog is never NULL

2011-08-01 Thread Ian Romanick
From: Ian Romanick ian.d.roman...@intel.com

This prevents assertion failures in ralloc_strcat.  The ralloc_free in
_mesa_free_shader_program_data can be omitted because freeing the
gl_shader_program in _mesa_delete_shader_program will take care of
this automatically.

A bunch of this code could use a refactor to use ralloc a bit more
effectively.  A bunch of the things that are allocated with malloc and
owned by the gl_shader_program should be allocated with ralloc (using
the gl_shader_program as the context).
---
 src/glsl/main.cpp |1 +
 src/mesa/main/shaderobj.c |   11 ++-
 2 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/src/glsl/main.cpp b/src/glsl/main.cpp
index 9f85096..9b8a507 100644
--- a/src/glsl/main.cpp
+++ b/src/glsl/main.cpp
@@ -221,6 +221,7 @@ main(int argc, char **argv)
 
whole_program = rzalloc (NULL, struct gl_shader_program);
assert(whole_program != NULL);
+   whole_program-InfoLog = ralloc_strdup(whole_program, );
 
for (/* empty */; argc  optind; optind++) {
   whole_program-Shaders =
diff --git a/src/mesa/main/shaderobj.c b/src/mesa/main/shaderobj.c
index 33d91ad..f128648 100644
--- a/src/mesa/main/shaderobj.c
+++ b/src/mesa/main/shaderobj.c
@@ -244,6 +244,8 @@ _mesa_init_shader_program(struct gl_context *ctx, struct 
gl_shader_program *prog
prog-Geom.InputType = GL_TRIANGLES;
prog-Geom.OutputType = GL_TRIANGLE_STRIP;
 #endif
+
+   prog-InfoLog = ralloc_strdup(prog, );
 }
 
 /**
@@ -283,6 +285,10 @@ _mesa_clear_shader_program_data(struct gl_context *ctx,
   _mesa_free_parameter_list(shProg-Varying);
   shProg-Varying = NULL;
}
+
+   assert(shProg-InfoLog != NULL);
+   ralloc_free(shProg-InfoLog);
+   shProg-InfoLog = ralloc_strdup(shProg, );
 }
 
 
@@ -317,11 +323,6 @@ _mesa_free_shader_program_data(struct gl_context *ctx,
   shProg-Shaders = NULL;
}
 
-   if (shProg-InfoLog) {
-  ralloc_free(shProg-InfoLog);
-  shProg-InfoLog = NULL;
-   }
-
/* Transform feedback varying vars */
for (i = 0; i  shProg-TransformFeedback.NumVarying; i++) {
   free(shProg-TransformFeedback.VaryingNames[i]);
-- 
1.7.4.4

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


[Mesa-dev] [PATCH 4/7] ir_to_mesa: Use Add linker_error_printf instead of fail_link

2011-08-01 Thread Ian Romanick
From: Ian Romanick ian.d.roman...@intel.com

The functions were almost identical.
---
 src/mesa/program/ir_to_mesa.cpp |   56 +-
 1 files changed, 25 insertions(+), 31 deletions(-)

diff --git a/src/mesa/program/ir_to_mesa.cpp b/src/mesa/program/ir_to_mesa.cpp
index 8b4a535..3c553a5 100644
--- a/src/mesa/program/ir_to_mesa.cpp
+++ b/src/mesa/program/ir_to_mesa.cpp
@@ -331,20 +331,6 @@ dst_reg undef_dst = dst_reg(PROGRAM_UNDEFINED, 
SWIZZLE_NOOP);
 
 dst_reg address_reg = dst_reg(PROGRAM_ADDRESS, WRITEMASK_X);
 
-static void
-fail_link(struct gl_shader_program *prog, const char *fmt, ...) PRINTFLIKE(2, 
3);
-
-static void
-fail_link(struct gl_shader_program *prog, const char *fmt, ...)
-{
-   va_list args;
-   va_start(args, fmt);
-   ralloc_vasprintf_append(prog-InfoLog, fmt, args);
-   va_end(args);
-
-   prog-LinkStatus = GL_FALSE;
-}
-
 static int
 swizzle_for_size(int size)
 {
@@ -789,10 +775,11 @@ ir_to_mesa_visitor::visit(ir_variable *ir)
 
   if (storage-file == PROGRAM_TEMPORARY 
  dst.index != storage-index + (int) ir-num_state_slots) {
-fail_link(this-shader_program,
-  failed to load builtin uniform `%s'  (%d/%d regs loaded)\n,
-  ir-name, dst.index - storage-index,
-  type_size(ir-type));
+linker_error_printf(this-shader_program,
+failed to load builtin uniform `%s' 
+(%d/%d regs loaded)\n,
+ir-name, dst.index - storage-index,
+type_size(ir-type));
   }
}
 }
@@ -2413,29 +2400,35 @@ check_resources(const struct gl_context *ctx,
case GL_VERTEX_PROGRAM_ARB:
   if (_mesa_bitcount(prog-SamplersUsed) 
   ctx-Const.MaxVertexTextureImageUnits) {
- fail_link(shader_program, Too many vertex shader texture samplers);
+ linker_error_printf(shader_program,
+Too many vertex shader texture samplers);
   }
   if (prog-Parameters-NumParameters  MAX_UNIFORMS) {
- fail_link(shader_program, Too many vertex shader constants);
+ linker_error_printf(shader_program,
+Too many vertex shader constants);
   }
   break;
case MESA_GEOMETRY_PROGRAM:
   if (_mesa_bitcount(prog-SamplersUsed) 
   ctx-Const.MaxGeometryTextureImageUnits) {
- fail_link(shader_program, Too many geometry shader texture 
samplers);
+ linker_error_printf(shader_program,
+Too many geometry shader texture samplers);
   }
   if (prog-Parameters-NumParameters 
   MAX_GEOMETRY_UNIFORM_COMPONENTS / 4) {
- fail_link(shader_program, Too many geometry shader constants);
+ linker_error_printf(shader_program,
+Too many geometry shader constants);
   }
   break;
case GL_FRAGMENT_PROGRAM_ARB:
   if (_mesa_bitcount(prog-SamplersUsed) 
   ctx-Const.MaxTextureImageUnits) {
- fail_link(shader_program, Too many fragment shader texture 
samplers);
+ linker_error_printf(shader_program,
+Too many fragment shader texture samplers);
   }
   if (prog-Parameters-NumParameters  MAX_UNIFORMS) {
- fail_link(shader_program, Too many fragment shader constants);
+ linker_error_printf(shader_program,
+Too many fragment shader constants);
   }
   break;
default:
@@ -2550,9 +2543,10 @@ add_uniforms_to_parameters_list(struct gl_shader_program 
*shader_program,
  * from _mesa_add_uniform) has to match what the linker chose.
  */
 if (index != parameter_index) {
-   fail_link(shader_program, Allocation of uniform `%s' to target 
- failed (%d vs %d)\n,
- uniform-Name, index, parameter_index);
+   linker_error_printf(shader_program,
+   Allocation of uniform `%s' to target failed 
+   (%d vs %d)\n,
+   uniform-Name, index, parameter_index);
 }
   }
}
@@ -2585,8 +2579,8 @@ set_uniform_initializer(struct gl_context *ctx, void 
*mem_ctx,
int loc = _mesa_get_uniform_location(ctx, shader_program, name);
 
if (loc == -1) {
-  fail_link(shader_program,
-   Couldn't find uniform for initializer %s\n, name);
+  linker_error_printf(shader_program,
+ Couldn't find uniform for initializer %s\n, name);
   return;
}
 
@@ -2987,7 +2981,7 @@ get_mesa_program(struct gl_context *ctx,
 prog-IndirectRegisterFiles |= 1  mesa_inst-SrcReg[src].File;
 
   if (options-EmitNoIfs  mesa_inst-Opcode == OPCODE_IF) {
-fail_link(shader_program, Couldn't flatten if statement\n);
+linker_error_printf(shader_program, Couldn't flatten if statement\n);
   }
 
  

[Mesa-dev] [PATCH 5/7] ir_to_mesa: Emit warnings instead of errors for IR that can't be lowered

2011-08-01 Thread Ian Romanick
From: Ian Romanick ian.d.roman...@intel.com

Rely on the driver to do the right thing.  This probably means falling
back to software.  Page 88 of the OpenGL 2.1 spec specifically says:

A shader should not fail to compile, and a program object should
not fail to link due to lack of instruction space or lack of
temporary variables. Implementations should ensure that all valid
shaders and program objects may be successfully compiled, linked
and executed.

There is no provision for saying No to a valid shader that is
difficult for the hardware to handle, so stop doing that.

On i915 this causes a large number of piglit tests to change from FAIL
to WARN.  The warning is because the driver still emits messages to
stderr like i915_program_error: Unsupported opcode: BGNLOOP.

It also fixes ES2 conformance CorrectFull_frag and CorrectParse1_frag
on i915 (and probably other hardware that can't handle loops).
---
 src/mesa/program/ir_to_mesa.cpp |   28 
 1 files changed, 24 insertions(+), 4 deletions(-)

diff --git a/src/mesa/program/ir_to_mesa.cpp b/src/mesa/program/ir_to_mesa.cpp
index 3c553a5..83b96f5 100644
--- a/src/mesa/program/ir_to_mesa.cpp
+++ b/src/mesa/program/ir_to_mesa.cpp
@@ -2980,11 +2980,31 @@ get_mesa_program(struct gl_context *ctx,
  if (mesa_inst-SrcReg[src].RelAddr)
 prog-IndirectRegisterFiles |= 1  mesa_inst-SrcReg[src].File;
 
-  if (options-EmitNoIfs  mesa_inst-Opcode == OPCODE_IF) {
-linker_error_printf(shader_program, Couldn't flatten if statement\n);
-  }
-
   switch (mesa_inst-Opcode) {
+  case OPCODE_IF:
+if (options-EmitNoIfs) {
+   linker_warning_printf(shader_program,
+ Couldn't flatten if-statement.  
+ This will likely result in software 
+ rasterization.\n);
+}
+break;
+  case OPCODE_BGNLOOP:
+if (options-EmitNoLoops) {
+   linker_warning_printf(shader_program,
+ Couldn't unroll loop.  
+ This will likely result in software 
+ rasterization.\n);
+}
+break;
+  case OPCODE_CONT:
+if (options-EmitNoCont) {
+   linker_warning_printf(shader_program,
+ Couldn't lower continue-statement.  
+ This will likely result in software 
+ rasterization.\n);
+}
+break;
   case OPCODE_BGNSUB:
 inst-function-inst = i;
 mesa_inst-Comment = strdup(inst-function-sig-function_name());
-- 
1.7.4.4

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


[Mesa-dev] [PATCH 6/7] i915: Fail without crashing if a Mesa IR program uses too many registers

2011-08-01 Thread Ian Romanick
From: Ian Romanick ian.d.roman...@intel.com

So far this can only happen in GLSL shaders that contain flow-control
that could not be lowered.  These programs would have failed to run on
hardware anyway.
---
 src/mesa/drivers/dri/i915/i915_fragprog.c |   15 +--
 1 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/src/mesa/drivers/dri/i915/i915_fragprog.c 
b/src/mesa/drivers/dri/i915/i915_fragprog.c
index 6e1d709..32050ce 100644
--- a/src/mesa/drivers/dri/i915/i915_fragprog.c
+++ b/src/mesa/drivers/dri/i915/i915_fragprog.c
@@ -303,7 +303,7 @@ do {
\
 /* 
  * TODO: consider moving this into core 
  */
-static void calc_live_regs( struct i915_fragment_program *p )
+static bool calc_live_regs( struct i915_fragment_program *p )
 {
 const struct gl_fragment_program *program = p-FragProg;
 GLuint regsUsed = 0x;
@@ -317,6 +317,9 @@ static void calc_live_regs( struct i915_fragment_program *p 
)
 
 /* Register is written to: unmark as live for this and preceeding ops 
*/ 
 if (inst-DstReg.File == PROGRAM_TEMPORARY) {
+   if (inst-DstReg.Index  16)
+  return false;
+
 live_components[inst-DstReg.Index] = ~inst-DstReg.WriteMask;
 if (live_components[inst-DstReg.Index] == 0)
 regsUsed = ~(1  inst-DstReg.Index);
@@ -327,6 +330,9 @@ static void calc_live_regs( struct i915_fragment_program *p 
)
 if (inst-SrcReg[a].File == PROGRAM_TEMPORARY) {
 unsigned c;
 
+   if (inst-SrcReg[a].Index  16)
+  return false;
+
 regsUsed |= 1  inst-SrcReg[a].Index;
 
 for (c = 0; c  4; c++) {
@@ -340,6 +346,8 @@ static void calc_live_regs( struct i915_fragment_program *p 
)
 
 p-usedRegs[i] = regsUsed;
 }
+
+return true;
 }
 
 static GLuint get_live_regs( struct i915_fragment_program *p, 
@@ -394,7 +402,10 @@ upload_program(struct i915_fragment_program *p)
 
/* Not always needed:
 */
-   calc_live_regs(p);
+   if (!calc_live_regs(p)) {
+  i915_program_error(p, Could not allocate registers);
+  return;
+   }
 
while (1) {
   GLuint src0, src1, src2, flags;
-- 
1.7.4.4

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


[Mesa-dev] [PATCH 7/7] i915: Only emit program errors when INTEL_DEBUG=wm

2011-08-01 Thread Ian Romanick
From: Ian Romanick ian.d.roman...@intel.com

This makes piglit a lot more happy.
---
 src/mesa/drivers/dri/i915/i915_program.c |   14 --
 1 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/src/mesa/drivers/dri/i915/i915_program.c 
b/src/mesa/drivers/dri/i915/i915_program.c
index ca1949b..304ceec 100644
--- a/src/mesa/drivers/dri/i915/i915_program.c
+++ b/src/mesa/drivers/dri/i915/i915_program.c
@@ -442,14 +442,16 @@ i915_emit_param4fv(struct i915_fragment_program * p, 
const GLfloat * values)
 void
 i915_program_error(struct i915_fragment_program *p, const char *fmt, ...)
 {
-   va_list args;
+   if (unlikely(INTEL_DEBUG  DEBUG_WM)) {
+  va_list args;
 
-   fprintf(stderr, i915_program_error: );
-   va_start(args, fmt);
-   vfprintf(stderr, fmt, args);
-   va_end(args);
+  fprintf(stderr, i915_program_error: );
+  va_start(args, fmt);
+  vfprintf(stderr, fmt, args);
+  va_end(args);
 
-   fprintf(stderr, \n);
+  fprintf(stderr, \n);
+   }
p-error = 1;
 }
 
-- 
1.7.4.4

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


Re: [Mesa-dev] [Mesa3d-dev] patches for various problems when playing with egl

2011-08-01 Thread Ian Romanick
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

On 08/01/2011 10:40 AM, Ian Romanick wrote:
 On 07/31/2011 02:51 PM, kristof.ralov...@gmail.com wrote:
 Please apply!
 
 Send patches using git-send-email.  People cannot reply with review
 comments to patches sent as attachments.

And don't use mesa3d-...@lists.sourceforge.net.  That list is dead.  Use
mesa-...@freedesktop.org.

 patch 1:
 
 +   if (!gbm)
 +   {
 +  free(dri2_dpy);
 +  return EGL_FALSE;
 +   }
 
 should be
 
 +   if (!gbm) {
 +  free(dri2_dpy);
 +  return EGL_FALSE;
 +   }
 
 I think 'gbm == NULL' is the more common idiom in Mesa, but either way
 works for most people.
 
 It may be even better to fold this hunk with the following if-statement:
 
 gbm = (struct gbm_device *) disp-PlatformDisplay;
 -   if (strcmp(gbm_device_get_backend_name(gbm), drm) != 0) {
 +   if (!gbm || strcmp(gbm_device_get_backend_name(gbm), drm) != 0) {
free(dri2_dpy);
return EGL_FALSE;
 
 patch 2:
 
 I don't think always linking with -lm is quite correct either.  The
 comment at the top says that backends should define GBM_LIBS, but then
 nothing under src/gdb uses GBM_LIBS.
 
 Also, I think everyone hates the commit message style of starting a
 sentence in the brief line and continuing it in the body.  A better
 message is:
 
 gbm: link gbm_gallium_drm.so against math library
 
 This avoids the following runtime error with EGL on platforms that
 require linking with libm for nontrivial math functions:
 
 failed to load module: /xorg/lib64/gbm/gbm_gallium_drm.so: undefined
 symbol: powf
-BEGIN PGP SIGNATURE-
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/

iEYEARECAAYFAk425TMACgkQX1gOwKyEAw8/4gCgg75nOjsK/QAjPxQG5kWc6ZB8
eE8An2auNAnwbn3HrswuGNBs/0WR6mqG
=1HFY
-END PGP SIGNATURE-
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 7/7] i915: Only emit program errors when INTEL_DEBUG=wm

2011-08-01 Thread Eric Anholt
On Mon,  1 Aug 2011 10:20:23 -0700, Ian Romanick i...@freedesktop.org wrote:
 From: Ian Romanick ian.d.roman...@intel.com
 
 This makes piglit a lot more happy.

I'd include DEBUG_FALLBACK, too.  That's what's about to happen when
someone uses the program, and when people are hitting swrast we tell
them to look at INTEL_DEBUG=fall.


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


Re: [Mesa-dev] [PATCH 6/6] ir_to_mesa: Try to avoid emitting a MOV_SAT to saturate an expression tree.

2011-08-01 Thread Ian Romanick
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

On 07/29/2011 12:48 PM, Eric Anholt wrote:
 Fixes a regression in codegen quality for ff_fragment_shader
 conversion to GLSL -- glean texCombine produces 7.5% fewer Mesa IR
 instructions.
 ---
  src/mesa/program/ir_to_mesa.cpp |   28 
  1 files changed, 24 insertions(+), 4 deletions(-)
 
 diff --git a/src/mesa/program/ir_to_mesa.cpp b/src/mesa/program/ir_to_mesa.cpp
 index 8b4a535..f82b048 100644
 --- a/src/mesa/program/ir_to_mesa.cpp
 +++ b/src/mesa/program/ir_to_mesa.cpp
 @@ -925,13 +925,33 @@ ir_to_mesa_visitor::try_emit_sat(ir_expression *ir)
 if (!sat_src)
return false;
  
 +   ir_to_mesa_instruction *new_inst;
 +

Why not move this declaration down to the one place new_inst is set?

 sat_src-accept(this);
 src_reg src = this-result;
  
 -   this-result = get_temp(ir-type);
 -   ir_to_mesa_instruction *inst;
 -   inst = emit(ir, OPCODE_MOV, dst_reg(this-result), src);
 -   inst-saturate = true;
 +   /* If we generated an expression instruction into a temporary in
 +* processing the saturate's operand, apply the saturate to that
 +* instruction.  Otherwise, generate a MOV to do the saturate.
 +*
 +* Note that we have to be careful to only do this optimization if
 +* the instruction in question was in generating src-result -- for
 +* example, ir_dereference_array would generate a MUL to create the
 +* reladdr and return us a src reg using that reladdr, and that MUL
 +* not the thing we're trying to saturate.

The and that MUL.. bit doesn't parse for me.

 +*/
 +   ir_expression *sat_src_expr = sat_src-as_expression();
 +   new_inst = (ir_to_mesa_instruction *)this-instructions.get_tail();
 +   if (sat_src_expr  (sat_src_expr-operation == ir_binop_mul ||
 + sat_src_expr-operation == ir_binop_add ||
 + sat_src_expr-operation == ir_binop_dot)) {
 +  new_inst-saturate = true;
 +   } else {
 +  this-result = get_temp(ir-type);
 +  ir_to_mesa_instruction *inst;
 +  inst = emit(ir, OPCODE_MOV, dst_reg(this-result), src);
 +  inst-saturate = true;
 +   }
  
 return true;
  }
-BEGIN PGP SIGNATURE-
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/

iEYEARECAAYFAk4253EACgkQX1gOwKyEAw+v8wCdEYODs+zLOoHkv1H1xmoU3UMX
NPkAoIXJCEAbKy/6F9lZFwvrArwVMHCc
=DJ3g
-END PGP SIGNATURE-
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] S2TC - yet another attempt to solve the S3TC issue

2011-08-01 Thread Rudolf Polzer
Hi,

I developed, together with Maik Merten, a replacement for S3TC with the
following properties:

- does not use any interesting algorithms (no color ramps, each 4x4 block is
  just a 2 colors palette - basically, this is Color Cell Compression from
  August 1986)
- is perfectly decodable by any S3TC decoder
- can somewhat (at a noticeable quality loss) attempt to decode S3TC
- due to the simpler nature of the algorithm, it outperforms the S3TC
  libtxc_dxtn
- due to the compression being simpler, it was easier to write a good
  compression algorithm for it, and it sometimes yields better quality than
  NVidia Texture Tools encoding to full S3TC

It is called S2TC because each block only has 2 colors, and expands to Super
Simple Texture Compression.

It comes in form of a libtxc_dxtn replacement library, so it can be used with
Mesa right now.

You can find more info on

https://github.com/divVerent/s2tc/wiki

including quality comparison pictures that compare it to full S3TC.

When used in conjunction with an OpenGL driver, this would mean:

- when a precompressed texture is uploaded, nothing changes - it is uploaded as
  full S3TC
- when an uncompressed texture is uploaded as a compressed format, it is 
converted
  using the S2TC encoder, which typically yields less quality than a S3TC 
encoder,
  but still renders correctly and usually good enough (see screenshots on my
  wiki)
- in case a software fallback hits (or llvmpipe is used), S2TC is used for
  decoding - due to some bit values only being defined in the full S3TC format
  spec, this will be somewhat wrong and yields reduced quality when 
precompressed
  S3TC textures are used
- I believe this suffices to claim support for GL_EXT_texture_compression_s3tc
  without having an actual S3TC compressor, as compressing to a sub-format of
  S3TC is the same as just having a less clever S3TC compressor. Decompressing 
being
  incorrect (but still good enough for most uses) in case of software fallbacks
  however may be a problem, but then one could still instead upload it to the
  GPU, let it decode it, and download the decoded texture instead

The question now is:

- does Mesa have any interest in integrating this method (doesn't have to be my
  reference implementation, which implements quite many selectable variations
  of the encoding methods), and still using full S3TC if a libtxc_dxtn is found?
- or would Mesa be interested in linking to this implementation as an 
alternative
  to full libtxc_dxtn especially for the USA, as it's likely that less
  licenses are required to use this method? One possible implementation BTW 
could
  be Linux distros shipping S2TC libtxc_dxtn by default, and providing a 
simple
  way for users to upgrade to a full S3TC library, if they are aware of the 
issues
  with it and see themselves not affected by them?

Best regards,

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


[Mesa-dev] Status of the GLSL-TGSI translator, part 2

2011-08-01 Thread Bryan Cain
Since Mesa 7.11 is released now, I figure it's time to discuss merging
the glsl-to-tgsi branch to master again.  The translator is more mature
than last time.  There are no regressions that I know of on any driver. 
The code generation has improved so that it's the same as or better than
ir_to_mesa in almost every case.

It also will still emit TGSI integer opcodes if you change
ctx-GLSLVersion from 120 to 130 in st_extensions.c.  Driver developers
can use this to implement these opcodes in their drivers, since they
will be needed for GLSL 1.30 eventually.

Are there any objections to merging it to master now?  If there aren't,
I'll revise some of the commit messages for correctness and wrap long
lines since cgit doesn't do that automatically, then merge it after
getting approval.

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


Re: [Mesa-dev] [PATCH 6/7] i915: Fail without crashing if a Mesa IR program uses too many registers

2011-08-01 Thread Eric Anholt
On Mon,  1 Aug 2011 10:20:22 -0700, Ian Romanick i...@freedesktop.org wrote:
 From: Ian Romanick ian.d.roman...@intel.com
 
 So far this can only happen in GLSL shaders that contain flow-control
 that could not be lowered.  These programs would have failed to run on
 hardware anyway.

This looks reasonable, but I don't understand why this can only happen
with flow control.  Couldn't we just have something with too many temps
involved without flow control?

 ---
  src/mesa/drivers/dri/i915/i915_fragprog.c |   15 +--
  1 files changed, 13 insertions(+), 2 deletions(-)
 
 diff --git a/src/mesa/drivers/dri/i915/i915_fragprog.c 
 b/src/mesa/drivers/dri/i915/i915_fragprog.c
 index 6e1d709..32050ce 100644
 --- a/src/mesa/drivers/dri/i915/i915_fragprog.c
 +++ b/src/mesa/drivers/dri/i915/i915_fragprog.c
 @@ -303,7 +303,7 @@ do {  
 \
  /* 
   * TODO: consider moving this into core 
   */
 -static void calc_live_regs( struct i915_fragment_program *p )
 +static bool calc_live_regs( struct i915_fragment_program *p )
  {
  const struct gl_fragment_program *program = p-FragProg;
  GLuint regsUsed = 0x;
 @@ -317,6 +317,9 @@ static void calc_live_regs( struct i915_fragment_program 
 *p )
  
  /* Register is written to: unmark as live for this and preceeding 
 ops */ 
  if (inst-DstReg.File == PROGRAM_TEMPORARY) {
 + if (inst-DstReg.Index  16)
 +return false;
 +
  live_components[inst-DstReg.Index] = ~inst-DstReg.WriteMask;
  if (live_components[inst-DstReg.Index] == 0)
  regsUsed = ~(1  inst-DstReg.Index);
 @@ -327,6 +330,9 @@ static void calc_live_regs( struct i915_fragment_program 
 *p )
  if (inst-SrcReg[a].File == PROGRAM_TEMPORARY) {
  unsigned c;
  
 + if (inst-SrcReg[a].Index  16)
 +return false;
 +
  regsUsed |= 1  inst-SrcReg[a].Index;
  
  for (c = 0; c  4; c++) {
 @@ -340,6 +346,8 @@ static void calc_live_regs( struct i915_fragment_program 
 *p )
  
  p-usedRegs[i] = regsUsed;
  }
 +
 +return true;
  }
  
  static GLuint get_live_regs( struct i915_fragment_program *p, 
 @@ -394,7 +402,10 @@ upload_program(struct i915_fragment_program *p)
  
 /* Not always needed:
  */
 -   calc_live_regs(p);
 +   if (!calc_live_regs(p)) {
 +  i915_program_error(p, Could not allocate registers);
 +  return;
 +   }
  
 while (1) {
GLuint src0, src1, src2, flags;
 -- 
 1.7.4.4
 
 ___
 mesa-dev mailing list
 mesa-dev@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/mesa-dev


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


Re: [Mesa-dev] [PATCH] i965/gen5+: Fix incorrect miptree layout for non-power-of-two cubemaps.

2011-08-01 Thread Kenneth Graunke
On 07/31/2011 07:25 PM, Eric Anholt wrote:
 On Sat, 30 Jul 2011 21:35:52 -0700, Kenneth Graunke kenn...@whitecape.org 
 wrote:
 For power-of-two sizes, h0 == mt-height0 since it's already a multiple
 of two.  However, for NPOT, they're different; h1 should be computed
 based on the original size.

 Fixes piglit test cubemap npot and oglconform_31 test textureNPOT.

 NOTE: This is a candidate for stable release branches.

 Signed-off-by: Kenneth Graunke kenn...@whitecape.org
 ---
  src/mesa/drivers/dri/i965/brw_tex_layout.c |2 +-
  1 files changed, 1 insertions(+), 1 deletions(-)

 Note that the piglit test referenced isn't committed yet; I sent it to the
 piglit mailing list.

 diff --git a/src/mesa/drivers/dri/i965/brw_tex_layout.c 
 b/src/mesa/drivers/dri/i965/brw_tex_layout.c
 index f462f32..46a417a 100644
 --- a/src/mesa/drivers/dri/i965/brw_tex_layout.c
 +++ b/src/mesa/drivers/dri/i965/brw_tex_layout.c
 @@ -60,7 +60,7 @@ GLboolean brw_miptree_layout(struct intel_context *intel,
 * given in Volume 1 of the BSpec.
 */
h0 = ALIGN(mt-height0, align_h);
 -  h1 = ALIGN(minify(h0), align_h);
 +  h1 = ALIGN(minify(mt-height0), align_h);
qpitch = (h0 + h1 + (intel-gen = 7 ? 12 : 11) * align_h);
if (mt-compressed)
   qpitch /= 4;
 
 
 This looks wrong to me.  The height of a level L is ALIGN(height0  L,
 j) according to SNB PRM vol1, 6.17.3.1 Computing MIP level sizes.
 Note that our calculation of j is wrong for a bunch of hardware/format
 combos -- I wonder if that was the issue you were looking at?

Just for the record, I talked with Eric about this in person, and he
agreed that my patch is correct.  mt-height0 is H_0 (actual height) in
the PRM, while h0 is h_0 (aligned).  We were using the wrong one.  Also,
for the case I was looking at, j == 2, so that wasn't the issue.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH] i965/gen5+: Fix incorrect miptree layout for non-power-of-two cubemaps.

2011-08-01 Thread Eric Anholt
On Mon, 01 Aug 2011 13:15:45 -0700, Kenneth Graunke kenn...@whitecape.org 
wrote:
 On 07/31/2011 07:25 PM, Eric Anholt wrote:
  On Sat, 30 Jul 2011 21:35:52 -0700, Kenneth Graunke kenn...@whitecape.org 
  wrote:
  For power-of-two sizes, h0 == mt-height0 since it's already a multiple
  of two.  However, for NPOT, they're different; h1 should be computed
  based on the original size.
 
  Fixes piglit test cubemap npot and oglconform_31 test textureNPOT.
 
  NOTE: This is a candidate for stable release branches.
 
  Signed-off-by: Kenneth Graunke kenn...@whitecape.org
  ---
   src/mesa/drivers/dri/i965/brw_tex_layout.c |2 +-
   1 files changed, 1 insertions(+), 1 deletions(-)
 
  Note that the piglit test referenced isn't committed yet; I sent it to the
  piglit mailing list.
 
  diff --git a/src/mesa/drivers/dri/i965/brw_tex_layout.c 
  b/src/mesa/drivers/dri/i965/brw_tex_layout.c
  index f462f32..46a417a 100644
  --- a/src/mesa/drivers/dri/i965/brw_tex_layout.c
  +++ b/src/mesa/drivers/dri/i965/brw_tex_layout.c
  @@ -60,7 +60,7 @@ GLboolean brw_miptree_layout(struct intel_context *intel,
* given in Volume 1 of the BSpec.
*/
   h0 = ALIGN(mt-height0, align_h);
  -h1 = ALIGN(minify(h0), align_h);
  +h1 = ALIGN(minify(mt-height0), align_h);
   qpitch = (h0 + h1 + (intel-gen = 7 ? 12 : 11) * align_h);
 if (mt-compressed)
  qpitch /= 4;
  
  
  This looks wrong to me.  The height of a level L is ALIGN(height0  L,
  j) according to SNB PRM vol1, 6.17.3.1 Computing MIP level sizes.
  Note that our calculation of j is wrong for a bunch of hardware/format
  combos -- I wonder if that was the issue you were looking at?
 
 Just for the record, I talked with Eric about this in person, and he
 agreed that my patch is correct.  mt-height0 is H_0 (actual height) in
 the PRM, while h0 is h_0 (aligned).  We were using the wrong one.  Also,
 for the case I was looking at, j == 2, so that wasn't the issue.

More to the point, I was reading the patch as exactly reversed of what
it did.  So, I think that's a Reviewed-by :)


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


[Mesa-dev] [PATCH 1/4] Revert glsl: Skip processing the first function's body in do_dead_functions().

2011-08-01 Thread Paul Berry
opt_dead_functions contained a shortcut to skip processing the first
function's body, based on the assumption that IR functions are
topologically sorted, with callees always coming before their callers
(therefore the first function cannot contain any calls).

This assumption turns out not to be true in general.  For example, the
following code snippet gets translated to IR that violates this
assumption:

void f();
void g();
void f() { g(); }
void g() { ... }

In practice, the shortcut didn't cause bugs because of a coincidence
of the circumstances in which opt_dead_functions is called:

(a) we do inlining right before dead function elimination, and
inlining (when successful) eliminates all calls.

(b) for user-defined functions, inlining is always successful, because
previous optimization passes (during compilation) have reduced
them to a form that is eligible for inlining.

(c) the function that appears first in the IR can't possibly call a
built-in function, because built-in functions are always emitted
before the function that calls them.

It seems unnecessarily fragile to have opt_dead_functions depend on
these coincidences.  And the next patch in this series will break (c).
So I'm reverting the shortcut.  The consequence will be a slight
increase in link time for complex shaders.

This reverts commit c75427f4c8767e131e5fb3de44fbc9d904cb992d.
---
 src/glsl/opt_dead_functions.cpp |   11 +--
 1 files changed, 1 insertions(+), 10 deletions(-)

diff --git a/src/glsl/opt_dead_functions.cpp b/src/glsl/opt_dead_functions.cpp
index 7c64c61..51c77e3 100644
--- a/src/glsl/opt_dead_functions.cpp
+++ b/src/glsl/opt_dead_functions.cpp
@@ -50,7 +50,6 @@ public:
ir_dead_functions_visitor()
{
   this-mem_ctx = ralloc_context(NULL);
-  this-seen_another_function_signature = false;
}
 
~ir_dead_functions_visitor()
@@ -65,8 +64,6 @@ public:
 
bool (*predicate)(ir_instruction *ir);
 
-   bool seen_another_function_signature;
-
/* List of signature_entry */
exec_list signature_list;
void *mem_ctx;
@@ -97,13 +94,7 @@ ir_dead_functions_visitor::visit_enter(ir_function_signature 
*ir)
   entry-used = true;
}
 
-   /* If this is the first signature to look at, no need to descend to see
-* if it has calls to another function signature.
-*/
-   if (!this-seen_another_function_signature) {
-  this-seen_another_function_signature = true;
-  return visit_continue_with_parent;
-   }
+
 
return visit_continue;
 }
-- 
1.7.6

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


[Mesa-dev] [PATCH 2/4] glsl: Emit function signatures at toplevel, even for built-ins.

2011-08-01 Thread Paul Berry
The ast-to-hir conversion needs to emit function signatures in two
circumstances: when a function declaration (or definition) is
encountered, and when a built-in function is encountered.

To avoid emitting a function signature in an illegal place (such as
inside a function), emit_function() checked whether we were inside a
function definition, and if so, emitted the signature before the
function definition.

However, this didn't cover the case of emitting function signatures
for built-in functions when those built-in functions are called from
global scope (e.g. a built-in function called from inside the constant
integer expression that specifies the length of an array).

This patch changes emit_function() so that it emits function
signatures at toplevel in all cases.
---
 src/glsl/ast.h|3 +--
 src/glsl/ast_function.cpp |2 +-
 src/glsl/ast_to_hir.cpp   |   31 ++-
 src/glsl/glsl_parser_extras.h |6 ++
 4 files changed, 22 insertions(+), 20 deletions(-)

diff --git a/src/glsl/ast.h b/src/glsl/ast.h
index 878f48b..d1de227 100644
--- a/src/glsl/ast.h
+++ b/src/glsl/ast.h
@@ -730,7 +730,6 @@ _mesa_ast_field_selection_to_hir(const ast_expression *expr,
 struct _mesa_glsl_parse_state *state);
 
 void
-emit_function(_mesa_glsl_parse_state *state, exec_list *instructions,
- ir_function *f);
+emit_function(_mesa_glsl_parse_state *state, ir_function *f);
 
 #endif /* AST_H */
diff --git a/src/glsl/ast_function.cpp b/src/glsl/ast_function.cpp
index 8bcf48d..34a82f8 100644
--- a/src/glsl/ast_function.cpp
+++ b/src/glsl/ast_function.cpp
@@ -125,7 +125,7 @@ match_function_by_name(exec_list *instructions, const char 
*name,
if (f == NULL) {
   f = new(ctx) ir_function(name);
   state-symbols-add_global_function(f);
-  emit_function(state, instructions, f);
+  emit_function(state, f);
}
 
f-add_signature(sig-clone_prototype(f, NULL));
diff --git a/src/glsl/ast_to_hir.cpp b/src/glsl/ast_to_hir.cpp
index c0524bf..c1f160e 100644
--- a/src/glsl/ast_to_hir.cpp
+++ b/src/glsl/ast_to_hir.cpp
@@ -66,6 +66,8 @@ _mesa_ast_to_hir(exec_list *instructions, struct 
_mesa_glsl_parse_state *state)
 
state-current_function = NULL;
 
+   state-toplevel_ir = instructions;
+
/* Section 4.2 of the GLSL 1.20 specification states:
 * The built-in functions are scoped in a scope outside the global scope
 *  users declare global variables in.  That is, a shader's global scope,
@@ -85,6 +87,8 @@ _mesa_ast_to_hir(exec_list *instructions, struct 
_mesa_glsl_parse_state *state)
   ast-hir(instructions, state);
 
detect_recursion_unlinked(state, instructions);
+
+   state-toplevel_ir = NULL;
 }
 
 
@@ -2926,23 +2930,16 @@ ast_parameter_declarator::parameters_to_hir(exec_list 
*ast_parameters,
 
 
 void
-emit_function(_mesa_glsl_parse_state *state, exec_list *instructions,
- ir_function *f)
+emit_function(_mesa_glsl_parse_state *state, ir_function *f)
 {
-   /* Emit the new function header */
-   if (state-current_function == NULL) {
-  instructions-push_tail(f);
-   } else {
-  /* IR invariants disallow function declarations or definitions nested
-   * within other function definitions.  Insert the new ir_function
-   * block in the instruction sequence before the ir_function block
-   * containing the current ir_function_signature.
-   */
-  ir_function *const curr =
-const_castir_function *(state-current_function-function());
-
-  curr-insert_before(f);
-   }
+   /* IR invariants disallow function declarations or definitions
+* nested within other function definitions.  But there is no
+* requirement about the relative order of function declarations
+* and definitions with respect to one another.  So simply insert
+* the new ir_function block at the end of the toplevel instruction
+* list.
+*/
+   state-toplevel_ir-push_tail(f);
 }
 
 
@@ -3069,7 +3066,7 @@ ast_function::hir(exec_list *instructions,
 return NULL;
   }
 
-  emit_function(state, instructions, f);
+  emit_function(state, f);
}
 
/* Verify the return type of main() */
diff --git a/src/glsl/glsl_parser_extras.h b/src/glsl/glsl_parser_extras.h
index 2f4d3cb..fc392da 100644
--- a/src/glsl/glsl_parser_extras.h
+++ b/src/glsl/glsl_parser_extras.h
@@ -129,6 +129,12 @@ struct _mesa_glsl_parse_state {
 */
class ir_function_signature *current_function;
 
+   /**
+* During AST to IR conversion, pointer to the toplevel IR
+* instruction list being generated.
+*/
+   exec_list *toplevel_ir;
+
/** Have we found a return statement in this function? */
bool found_return;
 
-- 
1.7.6

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


[Mesa-dev] [PATCH 3/4] glsl: Constant-fold built-in functions before outputting IR

2011-08-01 Thread Paul Berry
Rearranged the logic for converting the ast for a function call to
hir, so that we constant fold before emitting any IR.  Previously we
would emit some IR, and then only later detect whether we could
constant fold.  The unnecessary IR would usually get cleaned up by a
later optimization step, however in the case of a builtin function
being used to compute an array size, it was causing an assertion.

Fixes Piglit test array-size-constant-relational.vert.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=38625
---
 src/glsl/ast_function.cpp |   16 ++--
 1 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/src/glsl/ast_function.cpp b/src/glsl/ast_function.cpp
index 34a82f8..5b6ed3b 100644
--- a/src/glsl/ast_function.cpp
+++ b/src/glsl/ast_function.cpp
@@ -199,6 +199,20 @@ match_function_by_name(exec_list *instructions, const char 
*name,
*/
   ir_call *call = new(ctx) ir_call(sig, actual_parameters);
   if (!sig-return_type-is_void()) {
+ /* If the function call is a constant expression, don't
+  * generate the instructions to call it; just generate an
+  * ir_constant representing the constant value.
+  *
+  * Function calls can only be constant expressions starting
+  * in GLSL 1.20.
+  */
+ if (state-language_version = 120) {
+ir_constant *const_val = call-constant_expression_value();
+if (const_val) {
+   return const_val;
+}
+ }
+
 ir_variable *var;
 ir_dereference_variable *deref;
 
@@ -211,8 +225,6 @@ match_function_by_name(exec_list *instructions, const char 
*name,
 deref = new(ctx) ir_dereference_variable(var);
 ir_assignment *assign = new(ctx) ir_assignment(deref, call, NULL);
 instructions-push_tail(assign);
-if (state-language_version = 120)
-   var-constant_value = call-constant_expression_value();
 
 deref = new(ctx) ir_dereference_variable(var);
 return deref;
-- 
1.7.6

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


[Mesa-dev] [PATCH 4/4] glsl: Check array size is const before asserting that no IR was generated.

2011-08-01 Thread Paul Berry
process_array_type() contains an assertion to verify that no IR
instructions are generated while processing the expression that
specifies the size of the array.  This assertion needs to happen
_after_ checking whether the expression is constant.  Otherwise we may
crash on an illegal shader rather than reporting an error.

Fixes piglit tests array-size-non-builtin-function.vert and
array-size-with-side-effect.vert.
---
 src/glsl/ast_to_hir.cpp |   13 -
 1 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/src/glsl/ast_to_hir.cpp b/src/glsl/ast_to_hir.cpp
index c1f160e..3648236 100644
--- a/src/glsl/ast_to_hir.cpp
+++ b/src/glsl/ast_to_hir.cpp
@@ -1769,11 +1769,6 @@ process_array_type(YYLTYPE *loc, const glsl_type *base, 
ast_node *array_size,
   ir_rvalue *const ir = array_size-hir( dummy_instructions, state);
   YYLTYPE loc = array_size-get_location();
 
-  /* FINISHME: Verify that the grammar forbids side-effects in array
-   * FINISHME: sizes.   i.e., 'vec4 [x = 12] data'
-   */
-  assert(dummy_instructions.is_empty());
-
   if (ir != NULL) {
 if (!ir-type-is_integer()) {
_mesa_glsl_error( loc, state, array size must be integer type);
@@ -1790,6 +1785,14 @@ process_array_type(YYLTYPE *loc, const glsl_type *base, 
ast_node *array_size,
} else {
   assert(size-type == ir-type);
   length = size-value.u[0];
+
+   /* If the array size is const (and we've verified that
+* it is) then no instructions should have been emitted
+* when we converted it to HIR.  If they were emitted,
+* then either the array size isn't const after all, or
+* we are emitting unnecessary instructions.
+*/
+   assert(dummy_instructions.is_empty());
}
 }
   }
-- 
1.7.6

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


[Mesa-dev] [PATCH 0/5] glsl: Add switch statement support to the GLSL compiler.

2011-08-01 Thread Dan McCabe
This patch set adds support for switch statements to the GLSL compiler. We
modify the grammar for the compiler with productions for switch statements
and case labels, while adding supporting supporting productions not already 
present. New AST classes are defined to support those productions. However, 
with our approach no new IR is needed, allowing us to leverage all existing 
optimizations and code generation.

Regarding the grammar, we note that the grammar as summarized in the appendix 
of the GLSL specs leaves a bit to be desired. For example, it appears that case 
labels can be used anywhere a statement is valid. However, we note that the
description of the switch statement in Section 6.2 in the body of the spec is 
much more specific, and we follow that text to guide our creation of new
productions.

Specifically, we add productions for:
   switch body,
   case label list,
   case statement, and
   case statement list.
The switch body and the case statement allow us to limit where case labels may
be used.

In turn, we create new AST classes for each of these productions.

For code generation, we generate previously existing IR. Switch statements 
can be thought of a series of if/then/else statements. Case labels are 
compared with the value of a test expression and the case statements are 
executed if the comparison is true.

There are a couple of aspects of switch statements that complicate this 
simplistic view. The primary one is that cases can fall through sequentially
to subsequent cases, unless a break statement is encountered, in which case
the switch statement exits completely.

But break handling is further complicated by the fact that a break statement
can impact the exit of a loop. Thus, we need to coordinate break processing
between switch statements and loop statements.

The code generated by a switch statement maintains three temporary state
variables:
int test_value;
bool is_fallthru;
bool is_break;

test_value is initialized to the value of the test expression at the head of
the switch statement. This is the value that case labels are compared against.

is_fallthru is used to sequentially fall through to subsequent cases and is
initialized to false. When a case label matches the test expression, this
state variable is set to true. It will also be forced to false if a break
statement has been encountered. This forcing to false on break MUST be
after every case test. In practice, we defer that forcing to immediately after
the last case comparison prior to executing a case statement, but that is
an optimization.

is_break is used to indicate that a break statement has been executed and is
initialized to false. When a break statement is encountered, it is set to true.
This state variable is then used to conditionally force is_fallthru to to false
to prevent subsequent case statements from executing.

Code generation for break statements depends on whether the break statement is
inside a switch statement or inside a loop statement. If it inside a loop
statement is inside a break statement, the same code as before gets generated.
But if a switch statement is inside a loop statement, code is emitted to set
the is_break state to true.

Just as ASTs for loop statements are managed in a stack-like manner to handle 
nesting, we also add a bool to capture the innermost switch or loop condition. 
Note that we still need to maintain a loop AST stack to properly handle 
for-loop code generation on a continue statement. Technically, we don't (yet) 
need a switch AST stack, but we are using one for orthogonality with loop 
statements and in anticipation of potential future use. Note that a simple 
boolean stack would have sufficed.

We will illustrate a switch statement with its analogous conditional code that
a switch statement corresponds to by considering an example.

Consider the following switch statement:
switch (42) {
case 0:
case 1:
gl_FragColor = vec4(1.0, 2.0, 3.0, 4.0);
case 2:
case 3:
gl_FragColor = vec4(4.0, 3.0, 2.0, 1.0);
break;
case 4:
default:
gl_FragColor = vec4(0.0, 0.0, 0.0, 0.0);
}

Note that case 0 and case 1 fall through to cases 2 and 3 if they occur.

Note that case 4 and the default case must be reached explicitly, since cases
2 and 3 break at the end of their case.

Finally, note that case 4 and the default case don't break but simply fall
through to the end of the switch.

For this code, the equivalent code can be expressed as:
int test_val = 42; // capture value of test expression
bool is_fallthru = false; // prevent initial fall throughs
bool is_break = false; // capture the execution of a break stmt

is_fallthru |= (test_val == 0); // enable fallthru on case 0
is_fallthru |= (test_val == 1); // enable fallthru on case 1
is_fallthru = !is_break; // inhibit fallthru on previous break
   

[Mesa-dev] [PATCH 1/5] glsl: Create AST data structures for switch statement and case label

2011-08-01 Thread Dan McCabe
Data structures for switch statement and case label are created that parallel
the structure of other AST data.
---
 src/glsl/ast.h |   24 
 1 files changed, 20 insertions(+), 4 deletions(-)

diff --git a/src/glsl/ast.h b/src/glsl/ast.h
index 878f48b..2ee0b11 100644
--- a/src/glsl/ast.h
+++ b/src/glsl/ast.h
@@ -628,13 +628,19 @@ public:
 
 class ast_case_label : public ast_node {
 public:
+   ast_case_label(ast_expression *test_value);
+   virtual void print(void) const;
+
+   virtual ir_rvalue *hir(exec_list *instructions,
+ struct _mesa_glsl_parse_state *state);
 
/**
-* An expression of NULL means 'default'.
+* An test value of NULL means 'default'.
 */
-   ast_expression *expression;
+   ast_expression *test_value;
 };
 
+
 class ast_selection_statement : public ast_node {
 public:
ast_selection_statement(ast_expression *condition,
@@ -653,8 +659,18 @@ public:
 
 class ast_switch_statement : public ast_node {
 public:
-   ast_expression *expression;
-   exec_list statements;
+   ast_switch_statement(ast_expression *test_expression,
+   ast_node *body);
+   virtual void print(void) const;
+ 
+   virtual ir_rvalue *hir(exec_list *instructions,
+ struct _mesa_glsl_parse_state *state);
+
+   ast_expression *test_expression;
+   ast_node *body;
+
+protected:
+   void test_to_hir(exec_list *, struct _mesa_glsl_parse_state *);
 };
 
 class ast_iteration_statement : public ast_node {
-- 
1.7.4.1

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


[Mesa-dev] [PATCH 2/5] glsl: Add productions to GLSL grammar for switch statement

2011-08-01 Thread Dan McCabe
The grammar is modified to support switch statements. Rather than follow the
grammar in the appendix, which allows case labels to be placed ANYWHERE
as a regular statement, we follow the development of the grammar as
described in the body of the GLSL spec.

In this variation, the switch statement has a body which consists of a list
of case statements. A case statement is preceded by a list of case labels and
ends with a list of statements.
---
 src/glsl/glsl_parser.yy |   64 --
 1 files changed, 61 insertions(+), 3 deletions(-)

diff --git a/src/glsl/glsl_parser.yy b/src/glsl/glsl_parser.yy
index 2c0498e..b3727ce 100644
--- a/src/glsl/glsl_parser.yy
+++ b/src/glsl/glsl_parser.yy
@@ -206,6 +206,12 @@
 %type declaration struct_declarator_list
 %type node selection_statement
 %type selection_rest_statement selection_rest_statement
+%type node switch_statement
+%type node switch_body
+%type node case_label
+%type node case_label_list
+%type node case_statement
+%type node case_statement_list
 %type node iteration_statement
 %type node condition
 %type node conditionopt
@@ -1519,8 +1525,7 @@ simple_statement:
declaration_statement
| expression_statement
| selection_statement
-   | switch_statement  { $$ = NULL; }
-   | case_label{ $$ = NULL; }
+   | switch_statement
| iteration_statement
| jump_statement
;
@@ -1642,15 +1647,68 @@ condition:
}
;
 
+/*
+ * siwtch_statement grammar is based on the syntax described in the body
+ * of the GLSL spec, not in it's appendix!!!
+ */
 switch_statement:
-   SWITCH '(' expression ')' compound_statement
+   SWITCH '(' expression ')' switch_body
+   {
+  $$ = NULL;
+   }
+   ;
+
+switch_body:
+   '{' '}'
+   {
+  $$ = NULL;
+   }
+   | '{' case_statement_list '}'
+   {
+  $$ = NULL;
+   }
;
 
 case_label:
CASE expression ':'
+   {
+  $$ = NULL;
+   }
| DEFAULT ':'
+   {
+  $$ = NULL;
+   }
;
 
+case_label_list:
+   case_label
+   {
+  $$ = NULL;
+   }
+   | case_label_list case_label
+   {
+  $$ = NULL;
+   }
+   ;
+
+case_statement:
+   case_label_list statement_list
+   {
+  $$ = NULL;
+   }
+   ;
+
+case_statement_list:
+   case_statement
+   {
+  $$ = NULL;
+   }
+   | case_statement_list case_statement
+   {
+  $$ = NULL;
+   }
+   ;
+   
 iteration_statement:
WHILE '(' condition ')' statement_no_new_scope
{
-- 
1.7.4.1

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


[Mesa-dev] [PATCH 3/5] glsl: Create AST structs corresponding to new productions in grammar

2011-08-01 Thread Dan McCabe
Previously we added productions for:
switch_body
case_label_list
case_statement
case_statement_list
Now add AST structs corresponding to those productions.
---
 src/glsl/ast.h |   59 
 1 files changed, 59 insertions(+), 0 deletions(-)

diff --git a/src/glsl/ast.h b/src/glsl/ast.h
index 2ee0b11..6568f17 100644
--- a/src/glsl/ast.h
+++ b/src/glsl/ast.h
@@ -641,6 +641,65 @@ public:
 };
 
 
+class ast_case_label_list : public ast_node {
+public:
+   ast_case_label_list(void);
+   virtual void print(void) const;
+
+   virtual ir_rvalue *hir(exec_list *instructions,
+ struct _mesa_glsl_parse_state *state);
+
+   /**
+* A list of case labels.
+*/
+   exec_list labels;
+};
+
+
+class ast_case_statement : public ast_node {
+public:
+   ast_case_statement(ast_case_label_list *labels);
+   virtual void print(void) const;
+
+   virtual ir_rvalue *hir(exec_list *instructions,
+ struct _mesa_glsl_parse_state *state);
+
+   ast_case_label_list *labels;
+
+   /**
+* A list of statements.
+*/
+   exec_list stmts;
+};
+
+
+class ast_case_statement_list : public ast_node {
+public:
+   ast_case_statement_list(void);
+   virtual void print(void) const;
+
+   virtual ir_rvalue *hir(exec_list *instructions,
+ struct _mesa_glsl_parse_state *state);
+
+   /**
+* A list of cases.
+*/
+   exec_list cases;
+};
+
+
+class ast_switch_body : public ast_node {
+public:
+   ast_switch_body(ast_case_statement_list *stmts);
+   virtual void print(void) const;
+
+   virtual ir_rvalue *hir(exec_list *instructions,
+ struct _mesa_glsl_parse_state *state);
+
+   ast_case_statement_list *stmts;
+};
+
+
 class ast_selection_statement : public ast_node {
 public:
ast_selection_statement(ast_expression *condition,
-- 
1.7.4.1

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


[Mesa-dev] [PATCH 4/5] glsl: Reference data structure ctors in grammar

2011-08-01 Thread Dan McCabe
We now tie the grammar to the ctors of the ASTs they reference.

This requires that we actually have definitions of the ctors.

In addition, we also need to define print and hir methods for the AST
classes. The Print methods are pretty simple to flesh out. However, at this
stage of the development, we simply stub out the hir methods and flesh
them out later.

Also, since actual class instances get returned by the productions in the
grammar, we also need to designate the type of the productions that
reference those instances.
---
 src/glsl/ast_to_hir.cpp |   54 +
 src/glsl/glsl_parser.yy |   55 +++--
 src/glsl/glsl_parser_extras.cpp |  100 +++
 3 files changed, 193 insertions(+), 16 deletions(-)

diff --git a/src/glsl/ast_to_hir.cpp b/src/glsl/ast_to_hir.cpp
index c0524bf..db5b4ad 100644
--- a/src/glsl/ast_to_hir.cpp
+++ b/src/glsl/ast_to_hir.cpp
@@ -3319,6 +3319,60 @@ ast_selection_statement::hir(exec_list *instructions,
 }
 
 
+ir_rvalue *
+ast_switch_statement::hir(exec_list *instructions,
+ struct _mesa_glsl_parse_state *state)
+{
+   // FINISHME
+   return NULL;
+}
+
+
+ir_rvalue *
+ast_switch_body::hir(exec_list *instructions,
+ struct _mesa_glsl_parse_state *state)
+{
+   // FINISHME
+   return NULL;
+}
+
+
+ir_rvalue *
+ast_case_statement::hir(exec_list *instructions,
+ struct _mesa_glsl_parse_state *state)
+{
+   // FINISHME
+   return NULL;
+}
+
+
+ir_rvalue *
+ast_case_statement_list::hir(exec_list *instructions,
+ struct _mesa_glsl_parse_state *state)
+{
+   // FINISHME
+   return NULL;
+}
+
+
+ir_rvalue *
+ast_case_label::hir(exec_list *instructions,
+ struct _mesa_glsl_parse_state *state)
+{
+   // FINISHME
+   return NULL;
+}
+
+
+ir_rvalue *
+ast_case_label_list::hir(exec_list *instructions,
+ struct _mesa_glsl_parse_state *state)
+{
+   // FINISHME
+   return NULL;
+}
+
+
 void
 ast_iteration_statement::condition_to_hir(ir_loop *stmt,
  struct _mesa_glsl_parse_state *state)
diff --git a/src/glsl/glsl_parser.yy b/src/glsl/glsl_parser.yy
index b3727ce..fc3541c 100644
--- a/src/glsl/glsl_parser.yy
+++ b/src/glsl/glsl_parser.yy
@@ -67,6 +67,11 @@
ast_declarator_list *declarator_list;
ast_struct_specifier *struct_specifier;
ast_declaration *declaration;
+   ast_switch_body *switch_body;
+   ast_case_label *case_label;
+   ast_case_label_list *case_label_list;
+   ast_case_statement *case_statement;
+   ast_case_statement_list *case_statement_list;
 
struct {
   ast_node *cond;
@@ -207,11 +212,11 @@
 %type node selection_statement
 %type selection_rest_statement selection_rest_statement
 %type node switch_statement
-%type node switch_body
-%type node case_label
-%type node case_label_list
-%type node case_statement
-%type node case_statement_list
+%type switch_body switch_body
+%type case_label_list case_label_list
+%type case_label case_label
+%type case_statement case_statement
+%type case_statement_list case_statement_list
 %type node iteration_statement
 %type node condition
 %type node conditionopt
@@ -1654,58 +1659,76 @@ condition:
 switch_statement:
SWITCH '(' expression ')' switch_body
{
-  $$ = NULL;
+  $$ = new(state) ast_switch_statement($3, $5);
}
;
 
 switch_body:
'{' '}'
{
-  $$ = NULL;
+  $$ = new(state) ast_switch_body(NULL);
+  $$-set_location(yylloc);
}
| '{' case_statement_list '}'
{
-  $$ = NULL;
+  $$ = new(state) ast_switch_body($2);
+  $$-set_location(yylloc);
}
;
 
 case_label:
CASE expression ':'
{
-  $$ = NULL;
+  $$ = new(state) ast_case_label($2);
}
| DEFAULT ':'
{
-  $$ = NULL;
+  $$ = new(state) ast_case_label(NULL);
}
;
 
 case_label_list:
case_label
{
-  $$ = NULL;
+  ast_case_label_list *labels = new(state) ast_case_label_list();
+
+  labels-labels.push_tail( $1-link);
+  $$ = labels;
}
| case_label_list case_label
{
-  $$ = NULL;
+  $$ = $1;
+  $$-labels.push_tail( $2-link);
}
;
 
 case_statement:
-   case_label_list statement_list
+   case_label_list statement
{
-  $$ = NULL;
+  ast_case_statement *stmts = new(state) ast_case_statement($1);
+
+  stmts-stmts.push_tail( $2-link);
+  $$ = stmts
+   }
+   | case_statement statement
+   {
+  $$ = $1;
+  $$-stmts.push_tail( $2-link);
}
;
 
 case_statement_list:
case_statement
{
-  $$ = NULL;
+  ast_case_statement_list *cases= new(state) ast_case_statement_list();

[Mesa-dev] [PATCH 5/5] glsl: Generate IR for switch statements

2011-08-01 Thread Dan McCabe
Up until now modifying the GLSL compiler has been pretty straightforward.
This is where things get interesting. But still pretty straightforward.

Switch statements can be thought of a series of if/then/else statements.
Case labels are compared with the value of a test expression and the case
statements are executed if the comparison is true.

There are a couple of aspects of switch statements that complicate this simple
view of the world. The primary one is that cases can fall through sequentially
to subsequent case, unless a break statement is encountered, in which case,
the switch statement exits completely.

But break handling is further complicated by the fact that a break statement
can impact the exit of a loop. Thus, we need to coordinate break processing
between switch statements and loop statements.

The code generated by a switch statement maintains three temporary state
variables:
int test_value;
bool is_fallthru;
bool is_break;

test_value is initialized to the value of the test expression at the head of
the switch statement. This is the value that case labels are compared against.

is_fallthru is used to sequentially fall through to subsequent cases and is
initialized to false. When a case label matches the test expression, this
state variable is set to true. It will also be forced to false if a break
statement has been encountered. This forcing to false on break MUST be
after every case test. In practice, we defer that forcing to immediately after
the last case comparison prior to executing a case statement, but that is
an optimization.

is_break is used to indicate that a break statement has been executed and is
initialized to false. When a break statement is encountered, it is set to true.
This state variable is then used to conditionally force is_fallthru to to false
to prevent subsequent case statements from executing.

Code generation for break statements depends on whether the break statement is
inside a switch statement or inside a loop statement. If it inside a loop
statement is inside a break statement, the same code as before gets generated.
But if a switch statement is inside a loop statement, code is emitted to set
the is_break state to true.

Just as ASTs for loop statements are managed in a stack-like
manner to handle nesting, we also add a bool to capture the innermost switch
or loop condition. Note that we still need to maintain a loop AST stack to
properly handle for-loop code generation on a continue statement. Technically,
we don't (yet) need a switch AST stack, but I am using one for orthogonality
with loop statements, in anticipation of future use. Note that a simple
boolean stack would have sufficed.

We will illustrate a switch statement with its analogous conditional code that
a switch statement corresponds to by examining an example.

Consider the following switch statement:
switch (42) {
case 0:
case 1:
gl_FragColor = vec4(1.0, 2.0, 3.0, 4.0);
case 2:
case 3:
gl_FragColor = vec4(4.0, 3.0, 2.0, 1.0);
break;
case 4:
default:
gl_FragColor = vec4(0.0, 0.0, 0.0, 0.0);
}

Note that case 0 and case 1 fall through to cases 2 and 3 if they occur.

Note that case 4 and the default case must be reached explicitly, since cases
2 and 3 break at the end of their case.

Finally, note that case 4 and the default case don't break but simply fall
through to the end of the switch.

For this code, the equivalent code can be expressed as:
int test_val = 42; // capture value of test expression
bool is_fallthru = false; // prevent initial fall through
bool is_break = false; // capture the execution of a break stmt

is_fallthru |= (test_val == 0); // enable fallthru on case 0
is_fallthru |= (test_val == 1); // enable fallthru on case 1
is_fallthru = !is_break; // inhibit fallthru on previous break
if (is_fallthru) {
gl_FragColor = vec4(1.0, 2.0, 3.0, 4.0);
}

is_fallthru |= (test_val == 2); // enable fallthru on case 2
is_fallthru |= (test_val == 3); // enable fallthru on case 3
is_fallthru = !is_break; // inhibit fallthru on previous break
if (is_fallthru) {
gl_FragColor = vec4(4.0, 3.0, 2.0, 1.0);
is_break = true; // inhibit all subsequent fallthru for break
}

is_fallthru |= (test_val == 4); // enable fallthru on case 4
is_fallthru = true; // enable fallthru for default case
is_fallthru = !is_break; // inhibit fallthru on previous break
if (is_fallthru) {
gl_FragColor = vec4(0.0, 0.0, 0.0, 0.0);
}

The code generate for |= and = uses the conditional assignment capabilities
of the IR.
---
 src/glsl/ast_to_hir.cpp |  286 +--
 src/glsl/glsl_parser_extras.cpp |3 +-
 src/glsl/glsl_parser_extras.h   |   

Re: [Mesa-dev] [PATCH 1/4] Revert glsl: Skip processing the first function's body in do_dead_functions().

2011-08-01 Thread Ian Romanick
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

On 08/01/2011 04:07 PM, Paul Berry wrote:
 opt_dead_functions contained a shortcut to skip processing the first
 function's body, based on the assumption that IR functions are
 topologically sorted, with callees always coming before their callers
 (therefore the first function cannot contain any calls).

After linking, that is absolutely true.

When linking, we start with an empty shader.  Then we find main, and
pull it in.  For each function pulled in (initially just main), we
recursively pull in all the called functions.

In the absence of cycles (i.e., recursion), that should guarantee the
desired sort order.  Right?

 This assumption turns out not to be true in general.  For example, the
 following code snippet gets translated to IR that violates this
 assumption:
 
 void f();
 void g();
 void f() { g(); }
 void g() { ... }
 
 In practice, the shortcut didn't cause bugs because of a coincidence
 of the circumstances in which opt_dead_functions is called:
 
 (a) we do inlining right before dead function elimination, and
 inlining (when successful) eliminates all calls.
 
 (b) for user-defined functions, inlining is always successful, because
 previous optimization passes (during compilation) have reduced
 them to a form that is eligible for inlining.
 
 (c) the function that appears first in the IR can't possibly call a
 built-in function, because built-in functions are always emitted
 before the function that calls them.
 
 It seems unnecessarily fragile to have opt_dead_functions depend on
 these coincidences.  And the next patch in this series will break (c).

In any case, that is probably true.  I have this notion in the back of
mind for rewriting the function inliner.  As part of that, I've been
thinking about adding ir_function_signature::is_leaf for functions that
do not contain any calls.  The seen_another_function_signature flag here
is sort of a rough approximation of an is_leaf flag.  Using the is_leaf
flag instead should actually make performance better.

 So I'm reverting the shortcut.  The consequence will be a slight
 increase in link time for complex shaders.
 
 This reverts commit c75427f4c8767e131e5fb3de44fbc9d904cb992d.
 ---
  src/glsl/opt_dead_functions.cpp |   11 +--
  1 files changed, 1 insertions(+), 10 deletions(-)
 
 diff --git a/src/glsl/opt_dead_functions.cpp b/src/glsl/opt_dead_functions.cpp
 index 7c64c61..51c77e3 100644
 --- a/src/glsl/opt_dead_functions.cpp
 +++ b/src/glsl/opt_dead_functions.cpp
 @@ -50,7 +50,6 @@ public:
 ir_dead_functions_visitor()
 {
this-mem_ctx = ralloc_context(NULL);
 -  this-seen_another_function_signature = false;
 }
  
 ~ir_dead_functions_visitor()
 @@ -65,8 +64,6 @@ public:
  
 bool (*predicate)(ir_instruction *ir);
  
 -   bool seen_another_function_signature;
 -
 /* List of signature_entry */
 exec_list signature_list;
 void *mem_ctx;
 @@ -97,13 +94,7 @@ 
 ir_dead_functions_visitor::visit_enter(ir_function_signature *ir)
entry-used = true;
 }
  
 -   /* If this is the first signature to look at, no need to descend to see
 -* if it has calls to another function signature.
 -*/
 -   if (!this-seen_another_function_signature) {
 -  this-seen_another_function_signature = true;
 -  return visit_continue_with_parent;
 -   }
 +
  
 return visit_continue;
  }

-BEGIN PGP SIGNATURE-
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/

iEYEARECAAYFAk43RlkACgkQX1gOwKyEAw/8fACgmRSrAJpDwOHrKdpPgapzoaHZ
ShoAn3Suwp/ON23kYtYE6ORe3U3r5mF8
=Qyu8
-END PGP SIGNATURE-
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 2/4] glsl: Emit function signatures at toplevel, even for built-ins.

2011-08-01 Thread Ian Romanick
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

On 08/01/2011 04:07 PM, Paul Berry wrote:
 The ast-to-hir conversion needs to emit function signatures in two
 circumstances: when a function declaration (or definition) is
 encountered, and when a built-in function is encountered.
 
 To avoid emitting a function signature in an illegal place (such as
 inside a function), emit_function() checked whether we were inside a
 function definition, and if so, emitted the signature before the
 function definition.
 
 However, this didn't cover the case of emitting function signatures
 for built-in functions when those built-in functions are called from
 global scope (e.g. a built-in function called from inside the constant
 integer expression that specifies the length of an array).
 
 This patch changes emit_function() so that it emits function
 signatures at toplevel in all cases.

There's something about this patch that I don't grok.  The
state-current_function test in emit function exists specifically to
handle calls to functions (built-in or otherwise) at global scope.
Without that, code such as the snippet below would not work, and text in
the commit log seems to indicate that it shouldn't work.  However, I
verified that it does.

#version 120
const float x = cos(3.14159 / 2.0);

 ---
  src/glsl/ast.h|3 +--
  src/glsl/ast_function.cpp |2 +-
  src/glsl/ast_to_hir.cpp   |   31 ++-
  src/glsl/glsl_parser_extras.h |6 ++
  4 files changed, 22 insertions(+), 20 deletions(-)
 
 diff --git a/src/glsl/ast.h b/src/glsl/ast.h
 index 878f48b..d1de227 100644
 --- a/src/glsl/ast.h
 +++ b/src/glsl/ast.h
 @@ -730,7 +730,6 @@ _mesa_ast_field_selection_to_hir(const ast_expression 
 *expr,
struct _mesa_glsl_parse_state *state);
  
  void
 -emit_function(_mesa_glsl_parse_state *state, exec_list *instructions,
 -   ir_function *f);
 +emit_function(_mesa_glsl_parse_state *state, ir_function *f);
  
  #endif /* AST_H */
 diff --git a/src/glsl/ast_function.cpp b/src/glsl/ast_function.cpp
 index 8bcf48d..34a82f8 100644
 --- a/src/glsl/ast_function.cpp
 +++ b/src/glsl/ast_function.cpp
 @@ -125,7 +125,7 @@ match_function_by_name(exec_list *instructions, const 
 char *name,
   if (f == NULL) {
  f = new(ctx) ir_function(name);
  state-symbols-add_global_function(f);
 -emit_function(state, instructions, f);
 +emit_function(state, f);
   }
  
   f-add_signature(sig-clone_prototype(f, NULL));
 diff --git a/src/glsl/ast_to_hir.cpp b/src/glsl/ast_to_hir.cpp
 index c0524bf..c1f160e 100644
 --- a/src/glsl/ast_to_hir.cpp
 +++ b/src/glsl/ast_to_hir.cpp
 @@ -66,6 +66,8 @@ _mesa_ast_to_hir(exec_list *instructions, struct 
 _mesa_glsl_parse_state *state)
  
 state-current_function = NULL;
  
 +   state-toplevel_ir = instructions;
 +
 /* Section 4.2 of the GLSL 1.20 specification states:
  * The built-in functions are scoped in a scope outside the global scope
  *  users declare global variables in.  That is, a shader's global scope,
 @@ -85,6 +87,8 @@ _mesa_ast_to_hir(exec_list *instructions, struct 
 _mesa_glsl_parse_state *state)
ast-hir(instructions, state);
  
 detect_recursion_unlinked(state, instructions);
 +
 +   state-toplevel_ir = NULL;
  }
  
  
 @@ -2926,23 +2930,16 @@ ast_parameter_declarator::parameters_to_hir(exec_list 
 *ast_parameters,
  
  
  void
 -emit_function(_mesa_glsl_parse_state *state, exec_list *instructions,
 -   ir_function *f)
 +emit_function(_mesa_glsl_parse_state *state, ir_function *f)
  {
 -   /* Emit the new function header */
 -   if (state-current_function == NULL) {
 -  instructions-push_tail(f);
 -   } else {
 -  /* IR invariants disallow function declarations or definitions nested
 -   * within other function definitions.  Insert the new ir_function
 -   * block in the instruction sequence before the ir_function block
 -   * containing the current ir_function_signature.
 -   */
 -  ir_function *const curr =
 -  const_castir_function *(state-current_function-function());
 -
 -  curr-insert_before(f);
 -   }
 +   /* IR invariants disallow function declarations or definitions
 +* nested within other function definitions.  But there is no
 +* requirement about the relative order of function declarations
 +* and definitions with respect to one another.  So simply insert
 +* the new ir_function block at the end of the toplevel instruction
 +* list.
 +*/
 +   state-toplevel_ir-push_tail(f);
  }
  
  
 @@ -3069,7 +3066,7 @@ ast_function::hir(exec_list *instructions,
return NULL;
}
  
 -  emit_function(state, instructions, f);
 +  emit_function(state, f);
 }
  
 /* Verify the return type of main() */
 diff --git a/src/glsl/glsl_parser_extras.h b/src/glsl/glsl_parser_extras.h
 index 2f4d3cb..fc392da 100644
 --- 

Re: [Mesa-dev] [PATCH 6/7] i915: Fail without crashing if a Mesa IR program uses too many registers

2011-08-01 Thread Ian Romanick
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

On 08/01/2011 11:39 AM, Eric Anholt wrote:
 On Mon,  1 Aug 2011 10:20:22 -0700, Ian Romanick i...@freedesktop.org 
 wrote:
 From: Ian Romanick ian.d.roman...@intel.com

 So far this can only happen in GLSL shaders that contain flow-control
 that could not be lowered.  These programs would have failed to run on
 hardware anyway.
 
 This looks reasonable, but I don't understand why this can only happen
 with flow control.  Couldn't we just have something with too many temps
 involved without flow control?

I suppose the more correct comment would be I've only seen this occur
in GLSL shaders that contain flow-control that could not be lowered.  I
don't think it can happen outside GLSL because the assembly extensions
can reject (at the core Mesa level) programs that use too many temps.

How does this sound:

This can only happen in GLSL shaders because assembly shaders that use
too many temps are rejected by core Mesa.  It is easiest to make this
happen with shaders that contain flow-control that could not be lowered.

 ---
  src/mesa/drivers/dri/i915/i915_fragprog.c |   15 +--
  1 files changed, 13 insertions(+), 2 deletions(-)

 diff --git a/src/mesa/drivers/dri/i915/i915_fragprog.c 
 b/src/mesa/drivers/dri/i915/i915_fragprog.c
 index 6e1d709..32050ce 100644
 --- a/src/mesa/drivers/dri/i915/i915_fragprog.c
 +++ b/src/mesa/drivers/dri/i915/i915_fragprog.c
 @@ -303,7 +303,7 @@ do { 
 \
  /* 
   * TODO: consider moving this into core 
   */
 -static void calc_live_regs( struct i915_fragment_program *p )
 +static bool calc_live_regs( struct i915_fragment_program *p )
  {
  const struct gl_fragment_program *program = p-FragProg;
  GLuint regsUsed = 0x;
 @@ -317,6 +317,9 @@ static void calc_live_regs( struct i915_fragment_program 
 *p )
  
  /* Register is written to: unmark as live for this and preceeding 
 ops */ 
  if (inst-DstReg.File == PROGRAM_TEMPORARY) {
 +if (inst-DstReg.Index  16)
 +   return false;
 +
  live_components[inst-DstReg.Index] = ~inst-DstReg.WriteMask;
  if (live_components[inst-DstReg.Index] == 0)
  regsUsed = ~(1  inst-DstReg.Index);
 @@ -327,6 +330,9 @@ static void calc_live_regs( struct i915_fragment_program 
 *p )
  if (inst-SrcReg[a].File == PROGRAM_TEMPORARY) {
  unsigned c;
  
 +if (inst-SrcReg[a].Index  16)
 +   return false;
 +
  regsUsed |= 1  inst-SrcReg[a].Index;
  
  for (c = 0; c  4; c++) {
 @@ -340,6 +346,8 @@ static void calc_live_regs( struct i915_fragment_program 
 *p )
  
  p-usedRegs[i] = regsUsed;
  }
 +
 +return true;
  }
  
  static GLuint get_live_regs( struct i915_fragment_program *p, 
 @@ -394,7 +402,10 @@ upload_program(struct i915_fragment_program *p)
  
 /* Not always needed:
  */
 -   calc_live_regs(p);
 +   if (!calc_live_regs(p)) {
 +  i915_program_error(p, Could not allocate registers);
 +  return;
 +   }
  
 while (1) {
GLuint src0, src1, src2, flags;
 -- 
 1.7.4.4
-BEGIN PGP SIGNATURE-
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/

iEYEARECAAYFAk43UHYACgkQX1gOwKyEAw8PqwCcCh5n7385MCiXciB2kfbxMhoF
ruAAn1ezsWe0t0VYrKz7IDJ/MgW3F79K
=dExl
-END PGP SIGNATURE-
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 1/4] Revert glsl: Skip processing the first function's body in do_dead_functions().

2011-08-01 Thread Paul Berry
On 1 August 2011 17:35, Ian Romanick i...@freedesktop.org wrote:
 -BEGIN PGP SIGNED MESSAGE-
 Hash: SHA1

 On 08/01/2011 04:07 PM, Paul Berry wrote:
 opt_dead_functions contained a shortcut to skip processing the first
 function's body, based on the assumption that IR functions are
 topologically sorted, with callees always coming before their callers
 (therefore the first function cannot contain any calls).

 After linking, that is absolutely true.

 When linking, we start with an empty shader.  Then we find main, and
 pull it in.  For each function pulled in (initially just main), we
 recursively pull in all the called functions.

 In the absence of cycles (i.e., recursion), that should guarantee the
 desired sort order.  Right?

Hmm, what you say makes sense, but there must be something more subtle
going on, because what led me to make this patch was that I first
tried writing the rest of the patch series, and then when I tested it
I ran into problems because at link time, the functions weren't sorted
in callee-to-caller order.

I will investigate things further in the morning and let you know what I find.


 This assumption turns out not to be true in general.  For example, the
 following code snippet gets translated to IR that violates this
 assumption:

     void f();
     void g();
     void f() { g(); }
     void g() { ... }

 In practice, the shortcut didn't cause bugs because of a coincidence
 of the circumstances in which opt_dead_functions is called:

 (a) we do inlining right before dead function elimination, and
     inlining (when successful) eliminates all calls.

 (b) for user-defined functions, inlining is always successful, because
     previous optimization passes (during compilation) have reduced
     them to a form that is eligible for inlining.

 (c) the function that appears first in the IR can't possibly call a
     built-in function, because built-in functions are always emitted
     before the function that calls them.

 It seems unnecessarily fragile to have opt_dead_functions depend on
 these coincidences.  And the next patch in this series will break (c).

 In any case, that is probably true.  I have this notion in the back of
 mind for rewriting the function inliner.  As part of that, I've been
 thinking about adding ir_function_signature::is_leaf for functions that
 do not contain any calls.  The seen_another_function_signature flag here
 is sort of a rough approximation of an is_leaf flag.  Using the is_leaf
 flag instead should actually make performance better.

That's an intriguing idea, and it meshes well with some things Eric
was saying today, about how some of our optimization stages spend a
lot of time digging around the IR looking for something that we might
know a priori isn't there (e.g. if a shader never calls a texture
lookup function, there's no reason we would ever need to call
do_lower_texture_projection()).  Your is_leaf idea could be the first
of several optimizations of this kind.


 So I'm reverting the shortcut.  The consequence will be a slight
 increase in link time for complex shaders.

 This reverts commit c75427f4c8767e131e5fb3de44fbc9d904cb992d.
 ---
  src/glsl/opt_dead_functions.cpp |   11 +--
  1 files changed, 1 insertions(+), 10 deletions(-)

 diff --git a/src/glsl/opt_dead_functions.cpp 
 b/src/glsl/opt_dead_functions.cpp
 index 7c64c61..51c77e3 100644
 --- a/src/glsl/opt_dead_functions.cpp
 +++ b/src/glsl/opt_dead_functions.cpp
 @@ -50,7 +50,6 @@ public:
     ir_dead_functions_visitor()
     {
        this-mem_ctx = ralloc_context(NULL);
 -      this-seen_another_function_signature = false;
     }

     ~ir_dead_functions_visitor()
 @@ -65,8 +64,6 @@ public:

     bool (*predicate)(ir_instruction *ir);

 -   bool seen_another_function_signature;
 -
     /* List of signature_entry */
     exec_list signature_list;
     void *mem_ctx;
 @@ -97,13 +94,7 @@ 
 ir_dead_functions_visitor::visit_enter(ir_function_signature *ir)
        entry-used = true;
     }

 -   /* If this is the first signature to look at, no need to descend to see
 -    * if it has calls to another function signature.
 -    */
 -   if (!this-seen_another_function_signature) {
 -      this-seen_another_function_signature = true;
 -      return visit_continue_with_parent;
 -   }
 +

     return visit_continue;
  }

 -BEGIN PGP SIGNATURE-
 Version: GnuPG v1.4.11 (GNU/Linux)
 Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/

 iEYEARECAAYFAk43RlkACgkQX1gOwKyEAw/8fACgmRSrAJpDwOHrKdpPgapzoaHZ
 ShoAn3Suwp/ON23kYtYE6ORe3U3r5mF8
 =Qyu8
 -END PGP SIGNATURE-

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


Re: [Mesa-dev] Mesa (master): Fix PPC detection on darwin

2011-08-01 Thread Michel Dänzer
On Son, 2011-07-31 at 09:47 -0700, Jeremy Huddleston wrote: 
 Module: Mesa
 Branch: master
 Commit: e737a99a6fbafe3ba4b5175eea25d1598dbeb9d8
 URL:
 http://cgit.freedesktop.org/mesa/mesa/commit/?id=e737a99a6fbafe3ba4b5175eea25d1598dbeb9d8
 
 Author: Jeremy Huddleston jerem...@apple.com
 Date:   Sun Jul 31 09:21:56 2011 -0700
 
 Fix PPC detection on darwin
 
 Fixes regression introduced by 7004582c1894ede839c44e292b413fe4916d7e9e
 
 Signed-off-by: Jeremy Huddleston jerem...@apple.com
 
 ---
 
  src/gallium/include/pipe/p_config.h |4 ++--
  1 files changed, 2 insertions(+), 2 deletions(-)
 
 diff --git a/src/gallium/include/pipe/p_config.h 
 b/src/gallium/include/pipe/p_config.h
 index eea3d79..803b806 100644
 --- a/src/gallium/include/pipe/p_config.h
 +++ b/src/gallium/include/pipe/p_config.h
 @@ -99,9 +99,9 @@
  #endif
  #endif
  
 -#if defined(__PPC__)
 +#if defined(__ppc__) || defined(__ppc64__) || defined(__PPC__)
  #define PIPE_ARCH_PPC
 -#if defined(__PPC64__)
 +#if defined(__ppc64__) || defined(__PPC64__)
  #define PIPE_ARCH_PPC_64
  #endif
  #endif

This will result in both PIPE_ARCH_PPC and PIPE_ARCH_PPC_64 being
defined when __ppc64__ is defined. AFAICT the intention is for only one
PIPE_ARCH_* to be defined.


-- 
Earthling Michel Dänzer   |   http://www.amd.com
Libre software enthusiast |  Debian, X and DRI developer
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev