Re: [Mesa-dev] [PATCH 5/5] glsl: Clarify ir_function::matching_sigature()
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
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()
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
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
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
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
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
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
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
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
-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
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.
-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
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
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
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.
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.
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().
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.
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
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.
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.
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
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
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
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
Re: [Mesa-dev] [PATCH 1/4] Revert glsl: Skip processing the first function's body in do_dead_functions().
-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.
-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
-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().
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
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
