[Mesa-dev] [Bug 38705] New: call glDrawElements (GL_LINES, 2, GL_UNSIGNED_BYTE, indices); will be exhaust memory
https://bugs.freedesktop.org/show_bug.cgi?id=38705 Summary: call glDrawElements (GL_LINES, 2, GL_UNSIGNED_BYTE, indices); will be exhaust memory Product: Mesa Version: unspecified Platform: x86 (IA32) OS/Version: Linux (All) Status: NEW Severity: critical Priority: medium Component: Other AssignedTo: mesa-dev@lists.freedesktop.org ReportedBy: guyue@transmension.com i am in fedora15 Name: mesa-libGL Version : 7.11 Release : 0.11.20110525.0.fc15 Architecture: i686 Install Date: Sat 11 Jun 2011 09:25:32 AM CST Group : System Environment/Libraries Size: 577341 License : MIT Signature : RSA/SHA256, Wed 25 May 2011 02:50:50 PM CST, Key ID b4ebf579069c8460 Source RPM : mesa-7.11-0.11.20110525.0.fc15.src.rpm Build Date : Wed 25 May 2011 04:24:38 PM CST Build Host : x86-13.phx2.fedoraproject.org Relocations : (not relocatable) Packager: Fedora Project Vendor : Fedora Project URL : http://www.mesa3d.org Summary : Mesa libGL runtime libraries and DRI drivers Description : Mesa libGL runtime library in my application, when i call glDrawElements (GL_LINES, 2, GL_UNSIGNED_BYTE, indices); application will exhaust memory. -- 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
[Mesa-dev] [Bug 38705] call glDrawElements (GL_LINES, 2, GL_UNSIGNED_BYTE, indices); will be exhaust memory
https://bugs.freedesktop.org/show_bug.cgi?id=38705 Cedric Vivier cedr...@neonux.com changed: What|Removed |Added CC||cedr...@neonux.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 3/3] r600g: implement fragment and vertex color clamp
On Mon, Jun 27, 2011 at 8:38 AM, Roland Scheidegger srol...@vmware.com wrote: Am 25.06.2011 00:22, schrieb Vadim Girlin: On 06/24/2011 11:38 PM, Jerome Glisse wrote: On Fri, Jun 24, 2011 at 12:29 PM, Vadim Girlinvadimgir...@gmail.com wrote: Fixes https://bugs.freedesktop.org/show_bug.cgi?id=38440 Signed-off-by: Vadim Girlinvadimgir...@gmail.com As discussed previously, there is better to handle this. I think best solution is to always add the instruction and to conditionally execute them thanks to the boolean constant. If this reveal to have a too big impact on shader, other solution i see is adding a cf block with those instructions and to enable or disable that block (cf_nop) and reupload shader that would avoid a rebuild. I know its not optimal to do a full rebuild, but rebuild is needed only when the application will use the same shader in different clamping states. It won't be a problem if the application doesn't change clamping state or if it changes the state but uses each shader in one state only. So assuming that typical app will not use one shader in both states, it shouldn't be a problem. Is this assumption wrong? I'm not really sure because I have no much experience in this. But if it's wrong then it's probably better for performance to build and cache both versions. I tend to think you're right apps probably don't want to use the same shader both with and without clamping. Well if boolean block (see COND field set to SQ_CF_COND_BOOL in SQ_CF_WORD1) are free from perf point of view then i think it's best to have one shader with the clamp instruction inside the boolean enabled block. Only benchmark can tell. Cheers, Jerome ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 3/3] r600g: implement fragment and vertex color clamp
On Mon, Jun 27, 2011 at 2:38 PM, Roland Scheidegger srol...@vmware.com wrote: Am 25.06.2011 00:22, schrieb Vadim Girlin: On 06/24/2011 11:38 PM, Jerome Glisse wrote: On Fri, Jun 24, 2011 at 12:29 PM, Vadim Girlinvadimgir...@gmail.com wrote: Fixes https://bugs.freedesktop.org/show_bug.cgi?id=38440 Signed-off-by: Vadim Girlinvadimgir...@gmail.com As discussed previously, there is better to handle this. I think best solution is to always add the instruction and to conditionally execute them thanks to the boolean constant. If this reveal to have a too big impact on shader, other solution i see is adding a cf block with those instructions and to enable or disable that block (cf_nop) and reupload shader that would avoid a rebuild. I know its not optimal to do a full rebuild, but rebuild is needed only when the application will use the same shader in different clamping states. It won't be a problem if the application doesn't change clamping state or if it changes the state but uses each shader in one state only. So assuming that typical app will not use one shader in both states, it shouldn't be a problem. Is this assumption wrong? I'm not really sure because I have no much experience in this. But if it's wrong then it's probably better for performance to build and cache both versions. I tend to think you're right apps probably don't want to use the same shader both with and without clamping. It still can be changed by st/mesa or by u_blitter and u_blit for various reasons. IIRC, the OpenGL default is TRUE if the current framebuffer is fixed-point including texture_snorm and FALSE otherwise, so changing the framebuffer may change the clamp color state. Besides that, the u_blitter and u_blit operations always disable the clamping, so if a framebuffer is fixed-point and thus clamp color state is TRUE (if not changed by an app), the driver may receive state changes that turn the clamping on, off, on, off,... with the blit operations turning it off and everything else turning it on. The state might be changing pretty much all the time and doing full shader rebuilds repeatedly may turn some apps into a slideshow. Therefore we must ensure that a fragment shader is set/built as late as possible, i.e. in draw_vbo. Each shader variant should be compiled once at most and stored for later use. create_fs_state and bind_fs_state should not do anything except copying the parameters. Marek Also it seems last write to color output components is often done with op2 instructions, so there is another optimization possible - to find last op2 writes and set clamp bit instead of using additional instructions. Probably it's not very hard to implement with this patch. AFAICS it will be impossible with other suggested solutions. Such optimization should probably be part of a more generic optimization pass. Roland ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 3/3] r600g: implement fragment and vertex color clamp
On Mon, 2011-06-27 at 15:32 +0200, Marek Olšák wrote: On Mon, Jun 27, 2011 at 2:38 PM, Roland Scheidegger srol...@vmware.com wrote: Am 25.06.2011 00:22, schrieb Vadim Girlin: On 06/24/2011 11:38 PM, Jerome Glisse wrote: On Fri, Jun 24, 2011 at 12:29 PM, Vadim Girlinvadimgir...@gmail.com wrote: Fixes https://bugs.freedesktop.org/show_bug.cgi?id=38440 Signed-off-by: Vadim Girlinvadimgir...@gmail.com As discussed previously, there is better to handle this. I think best solution is to always add the instruction and to conditionally execute them thanks to the boolean constant. If this reveal to have a too big impact on shader, other solution i see is adding a cf block with those instructions and to enable or disable that block (cf_nop) and reupload shader that would avoid a rebuild. I know its not optimal to do a full rebuild, but rebuild is needed only when the application will use the same shader in different clamping states. It won't be a problem if the application doesn't change clamping state or if it changes the state but uses each shader in one state only. So assuming that typical app will not use one shader in both states, it shouldn't be a problem. Is this assumption wrong? I'm not really sure because I have no much experience in this. But if it's wrong then it's probably better for performance to build and cache both versions. I tend to think you're right apps probably don't want to use the same shader both with and without clamping. It still can be changed by st/mesa or by u_blitter and u_blit for various reasons. IIRC, the OpenGL default is TRUE if the current framebuffer is fixed-point including texture_snorm and FALSE otherwise, so changing the framebuffer may change the clamp color state. Besides that, the u_blitter and u_blit operations always disable the clamping, so if a framebuffer is fixed-point and thus clamp color state is TRUE (if not changed by an app), the driver may receive state changes that turn the clamping on, off, on, off,... with the blit operations turning it off and everything else turning it on. The state might be changing pretty much all the time and doing full shader rebuilds repeatedly may turn some apps into a slideshow. I haven't looked at the code, maybe this is irrelevant for some reason, but the alternative to doing rebuilds when this type of state changes is to permit 1 compiled version of the shader to exist, parameterized in different ways. That way the ping-pong scenario you describe results in swapping between shaders (which should be cheap), rather than rebuilding. Therefore we must ensure that a fragment shader is set/built as late as possible, i.e. in draw_vbo. Each shader variant should be compiled once at most and stored for later use. create_fs_state and bind_fs_state should not do anything except copying the parameters. Actually it sounds like you're describing the same idea here... Keith ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 3/3] r600g: implement fragment and vertex color clamp
Am 27.06.2011 16:04, schrieb Keith Whitwell: On Mon, 2011-06-27 at 15:32 +0200, Marek Olšák wrote: On Mon, Jun 27, 2011 at 2:38 PM, Roland Scheidegger srol...@vmware.com wrote: Am 25.06.2011 00:22, schrieb Vadim Girlin: On 06/24/2011 11:38 PM, Jerome Glisse wrote: On Fri, Jun 24, 2011 at 12:29 PM, Vadim Girlinvadimgir...@gmail.com wrote: Fixes https://bugs.freedesktop.org/show_bug.cgi?id=38440 Signed-off-by: Vadim Girlinvadimgir...@gmail.com As discussed previously, there is better to handle this. I think best solution is to always add the instruction and to conditionally execute them thanks to the boolean constant. If this reveal to have a too big impact on shader, other solution i see is adding a cf block with those instructions and to enable or disable that block (cf_nop) and reupload shader that would avoid a rebuild. I know its not optimal to do a full rebuild, but rebuild is needed only when the application will use the same shader in different clamping states. It won't be a problem if the application doesn't change clamping state or if it changes the state but uses each shader in one state only. So assuming that typical app will not use one shader in both states, it shouldn't be a problem. Is this assumption wrong? I'm not really sure because I have no much experience in this. But if it's wrong then it's probably better for performance to build and cache both versions. I tend to think you're right apps probably don't want to use the same shader both with and without clamping. It still can be changed by st/mesa or by u_blitter and u_blit for various reasons. IIRC, the OpenGL default is TRUE if the current framebuffer is fixed-point including texture_snorm and FALSE otherwise, so changing the framebuffer may change the clamp color state. Besides that, the u_blitter and u_blit operations always disable the clamping, so if a framebuffer is fixed-point and thus clamp color state is TRUE (if not changed by an app), the driver may receive state changes that turn the clamping on, off, on, off,... with the blit operations turning it off and everything else turning it on. The state might be changing pretty much all the time and doing full shader rebuilds repeatedly may turn some apps into a slideshow. I haven't looked at the code, maybe this is irrelevant for some reason, but the alternative to doing rebuilds when this type of state changes is to permit 1 compiled version of the shader to exist, parameterized in different ways. That way the ping-pong scenario you describe results in swapping between shaders (which should be cheap), rather than rebuilding. Therefore we must ensure that a fragment shader is set/built as late as possible, i.e. in draw_vbo. Each shader variant should be compiled once at most and stored for later use. create_fs_state and bind_fs_state should not do anything except copying the parameters. Actually it sounds like you're describing the same idea here... Keith I wasn't aware that the boolean block might be free in terms of performance. If so that might indeed be a good solution without needing to recompile. But there might be other cases which could benefit from cached shaders, so it looks both options have merit. Roland ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] st/mesa: fix overwriting gl_format with pipe_format since 9d380f48
fixes assert later on in texcompress2/r600g
---
src/mesa/state_tracker/st_cb_texture.c |4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/mesa/state_tracker/st_cb_texture.c
b/src/mesa/state_tracker/st_cb_texture.c
index d52e273..6907cfc 100644
--- a/src/mesa/state_tracker/st_cb_texture.c
+++ b/src/mesa/state_tracker/st_cb_texture.c
@@ -842,7 +842,7 @@ decompress_with_blit(struct gl_context * ctx, GLenum
target, GLint level,
else {
/* format translation via floats */
GLuint row;
- enum pipe_format format = util_format_linear(dst_texture-format);
+ enum pipe_format pformat = util_format_linear(dst_texture-format);
for (row = 0; row height; row++) {
const GLbitfield transferOps = 0x0; /* bypassed for glGetTexImage() */
GLfloat rgba[4 * MAX_WIDTH];
@@ -854,7 +854,7 @@ decompress_with_blit(struct gl_context * ctx, GLenum
target, GLint level,
/* get float[4] rgba row from surface */
pipe_get_tile_rgba_format(pipe, tex_xfer, 0, row, width, 1,
- format, rgba);
+ pformat, rgba);
_mesa_pack_rgba_span_float(ctx, width, (GLfloat (*)[4]) rgba, format,
type, dest, ctx-Pack, transferOps);
--
1.7.4.4
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 38699] Can't compile with d3d1x since commit 73df31eedd0f33c8a9907855cb247c8f87964c48
https://bugs.freedesktop.org/show_bug.cgi?id=38699 --- Comment #4 from Alexandre Demers alexandre.f.dem...@gmail.com 2011-06-27 08:09:27 PDT --- Still there with your suggestion. Should I open a different bug report? -- 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
[Mesa-dev] [Bug 38699] Can't compile with d3d1x since commit 73df31eedd0f33c8a9907855cb247c8f87964c48
https://bugs.freedesktop.org/show_bug.cgi?id=38699 --- Comment #5 from Chia-I Wu olva...@gmail.com 2011-06-27 08:35:02 PDT --- Yes. It could be that the system libEGL get picked for linking and it does not have the new symbol. Please also include the result of $ cd /home/dema1701/projects/mesa/mesa $ nm -D x86_64-linux-gnu/libEGL.so | grep _eglFilterConfigArray in the bug report. Thanks. -- 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
[Mesa-dev] [Bug 33758] CreateDRIDrawable can't fail gracefully
https://bugs.freedesktop.org/show_bug.cgi?id=33758 --- Comment #4 from nobled nob...@dreamwidth.org 2011-06-27 12:53:00 PDT --- Good news and bad news. Good news: I tested in an Ubuntu Maverick chroot, and I can't reproduce the segfault -- maybe it was fixed in Xlib? (In Lucid, libX11 is version 1.3.2; in Maverick, it's 1.3.3) Now, instead of silently segfaulting, it seems to call exit(1) after giving a much more informative printout. Which is a *very slight* improvement: (gdb) finish Run till exit from #0 drisw_bind_context (context=0x80690b0, old=0x4871e0, draw=106954753, read=106954753) at drisw_glx.c:267 X Error of failed request: BadDrawable (invalid Pixmap or Window parameter) Major opcode of failed request: 55 (X_CreateGC) Resource id in failed request: 0x661 Serial number of failed request: 26 Current serial number in output stream: 28 Program exited with code 01. (gdb) quit Bad news is, ajax's patch no longer applies cleanly to master 773556e0f537eba82d9d68d618e229140f413620, and I tested both of Chad's patches and the bug still happens. I'll attach a reduced testcase. -- 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
[Mesa-dev] [Bug 33758] CreateDRIDrawable can't fail gracefully
https://bugs.freedesktop.org/show_bug.cgi?id=33758 --- Comment #5 from nobled nob...@dreamwidth.org 2011-06-27 12:58:05 PDT --- Created an attachment (id=48483) -- (https://bugs.freedesktop.org/attachment.cgi?id=48483) small EGL program to produce BadDrawable error (Unrelatedly, I had to workaround another bug in the egl_glx driver by commenting out the EGL_RENDERABLE_TYPE line: otherwise, it failed to create the EGLSurface at all because it didn't recognize it: libEGL warning: bad surface attribute 0x3040) -- 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-RFC] autoconf: add --enable-{dri,glx,osmesa}
On 06/27/2011 12:17 PM, Dan Nicholson wrote:
On Sun, Jun 26, 2011 at 6:39 AM, Chia-I Wuolva...@gmail.com wrote:
From: Chia-I Wuo...@lunarg.com
The idea is that DRI driver, libGL and libOSMesa are libraries that can
be independently enabled, yet --with-driver does not allow us to easily
do that, if not impossible. This also matches what
--enable-{egl,xorg,d3d1x} do for the respective libraries.
I haven't read this in any detail, but I definitely like the idea.
When I originally wrote all this, I struggled to coordinate DRI vs.
GLX, and I didn't really bother with the EGL code that was mostly
experimental. This is much more coherently structured.
There are two libGL providers: Xlib-based and DRI-based. They cannot
coexist. To be able to choose between them, --enable-xlib-glx is also
added.
This is the only part that kind of bugs me. It seems to me that the
--enable-dri and --enable-xlib-glx options aren't really symmetric. I
believe you'd need this to be --enable-dri-glx to really act as a
provider. I can see why you didn't do that since dri is a provider
to many of the APIs and would require a lot more hacking of
configure.ac. Is my understanding of that correct? I'm not as familiar
with the newer non-GL Mesa components.
How does --enable-xcb (use XCB for GLX) fit in? I've been using that
for ages. Why isn't it the default these days? XCB isn't exactly
cutting edge, and I believe we already use it for some DRI2 stuff. Is
there some Xlib-based stuff that still needs to be converted? Can we
just kill Xlib-GLX entirely?
I'm probably not understanding any of this...sorry :)
--Kenneth
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 1/2] ir_to_mesa: Allocate temporary instructions on the visitor's ralloc context
From: Ian Romanick ian.d.roman...@intel.com
And don't delete them. Let ralloc clean them up. Deleting the
temporary IR leaves dangling references in the prog_instruction. That
results in a bad dereference when printing the IR with MESA_GLSL=dump.
NOTE: This is a candidate for the 7.10 and 7.11 branches.
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=38584
Reviewed-by: Eric Anholt e...@anholt.net
---
v2 incorporates review comments from Eric.
src/mesa/program/ir_to_mesa.cpp | 28
1 files changed, 12 insertions(+), 16 deletions(-)
diff --git a/src/mesa/program/ir_to_mesa.cpp b/src/mesa/program/ir_to_mesa.cpp
index 0086997..9389477 100644
--- a/src/mesa/program/ir_to_mesa.cpp
+++ b/src/mesa/program/ir_to_mesa.cpp
@@ -803,48 +803,44 @@ ir_to_mesa_visitor::visit(ir_loop *ir)
ir_dereference_variable *counter = NULL;
if (ir-counter != NULL)
- counter = new(ir) ir_dereference_variable(ir-counter);
+ counter = new(mem_ctx) ir_dereference_variable(ir-counter);
if (ir-from != NULL) {
assert(ir-counter != NULL);
- ir_assignment *a = new(ir) ir_assignment(counter, ir-from, NULL);
+ ir_assignment *a =
+ new(mem_ctx) ir_assignment(counter, ir-from, NULL);
a-accept(this);
- delete a;
}
emit(NULL, OPCODE_BGNLOOP);
if (ir-to) {
ir_expression *e =
-new(ir) ir_expression(ir-cmp, glsl_type::bool_type,
- counter, ir-to);
- ir_if *if_stmt = new(ir) ir_if(e);
+new(mem_ctx) ir_expression(ir-cmp, glsl_type::bool_type,
+ counter, ir-to);
+ ir_if *if_stmt = new(mem_ctx) ir_if(e);
- ir_loop_jump *brk = new(ir) ir_loop_jump(ir_loop_jump::jump_break);
+ ir_loop_jump *brk =
+ new(mem_ctx) ir_loop_jump(ir_loop_jump::jump_break);
if_stmt-then_instructions.push_tail(brk);
if_stmt-accept(this);
-
- delete if_stmt;
- delete e;
- delete brk;
}
visit_exec_list(ir-body_instructions, this);
if (ir-increment) {
ir_expression *e =
-new(ir) ir_expression(ir_binop_add, counter-type,
- counter, ir-increment);
+new(mem_ctx) ir_expression(ir_binop_add, counter-type,
+ counter, ir-increment);
- ir_assignment *a = new(ir) ir_assignment(counter, e, NULL);
+ ir_assignment *a =
+ new(mem_ctx) ir_assignment(counter, e, NULL);
a-accept(this);
- delete a;
- delete e;
}
emit(NULL, OPCODE_ENDLOOP);
--
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/2 v2] glsl: Don't choke when printing an anonymous function parameter
From: Ian Romanick ian.d.roman...@intel.com
NOTE: This is a candidate for the 7.10 and 7.11 branches.
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=38584
---
v2 actually fixes the bug. Based on feedback in the bug report.
src/glsl/ir_print_visitor.cpp | 10 ++
1 files changed, 10 insertions(+), 0 deletions(-)
diff --git a/src/glsl/ir_print_visitor.cpp b/src/glsl/ir_print_visitor.cpp
index 5b5409d..518910b 100644
--- a/src/glsl/ir_print_visitor.cpp
+++ b/src/glsl/ir_print_visitor.cpp
@@ -96,6 +96,16 @@ void ir_print_visitor::indent(void)
const char *
ir_print_visitor::unique_name(ir_variable *var)
{
+ /* var-name can be NULL in function prototypes when a type is given for a
+* parameter but no name is given. In that case, just return an empty
+* string. Don't worry about tracking the generated name in the printable
+* names hash because this is the only scope where it can ever appear.
+*/
+ if (var-name == NULL) {
+ static unsigned arg = 1;
+ return ralloc_asprintf(this-mem_ctx, parameter@%u, arg++);
+ }
+
/* Do we already have a name for this variable? */
const char *name = (const char *) hash_table_find(this-printable_names,
var);
if (name != NULL)
--
1.7.4.4
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 2/6] glsl: Add productions to GLSL grammar for switch statement
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On 06/24/2011 12:59 PM, Dan McCabe wrote: On 06/20/2011 03:39 PM, Ian Romanick wrote: -BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On 06/17/2011 05:43 PM, Dan McCabe wrote: 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. 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. Either this patch or patch 6/6 is the right place to restrict switch-statements and case labels to GLSL 1.30. From the looks of the series, these patches will merrily allow switch-statements on any GLSL version. That seems bad. :) Isn't that the purpose of the KYEWORD macro in glsl_lexer.ll ? Those macros already specify that that these are valid keywords in 1.3 or beyond already. Yes, of course. It's been some time since I've been in there. I forgot about that. :) -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.11 (GNU/Linux) Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/ iEYEARECAAYFAk4I9kgACgkQX1gOwKyEAw+YegCfRyGBwbIFTHmQDBwHxH9Ch0F0 +UAAnjtam9SBU9sHqUnV9+SOBQ6380XW =wnhg -END 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] glsl: Generate IR for switch statements
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1
On 06/24/2011 05:11 PM, Dan McCabe wrote:
On 06/24/2011 01:17 PM, Dan McCabe wrote:
On 06/20/2011 03:50 PM, Ian Romanick wrote:
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1
On 06/17/2011 05:43 PM, Dan McCabe wrote:
Beware! Here be dragons!
I think this will generate the wrong code for:
for (i = 0; i 10; i++) {
switch (i) {
default: continue;
}
}
It seems like that will generate some approximation of:
for (i = 0; i 10; i++) {
do {
continue;
break;
} while (true);
}
Right? This is why I had originally tracked loops and switch-statements
in a single stack. In this situation, you'd want to set a do a
continue flag in the switch-statement and emit a break (from the
switch). Something like:
for (i = 0; i 10; i++) {
bool do_cont = false;
do {
do_cont = true;
break;
break;
} while (true);
if (do_cont)
continue;
}
Yikes! Looks like you tripped over one of those dragons :(.
Using a do_cont variable (or similar device) feels kludgey to me. I've
been mulling this over for the last couple of days while I did other
things and I think the right way to do this might be to get rid of the
use of loop altogether.
But the devil is in the details, which I haven't worked out yet. Going
back to the unified loop/switch stack might be needed, though.
cheers, danm
All is not lost! (But you already knew that :)
One of the motivations of using the loop device is that it enabled the
relatively unmodified infrastructure for break statements. But let's get
rid of the break device and see where we go.
Without xlating switch stmts into loops, we can still retain the
test_val_tmp and is_fallthru_tmp temporaries (in fact, we need to;
nothing changes there since that is how case labels are managed).
However, the standard break processing no longer applies, since we are
not in a loop (for the switch stmt in any event). If we introduce a bool
temporary called is_break_tmp and initialize it to false, then when we
encounter a break stmt, we set is_break_tmp to true. Now we must guard
the case statement with both fallthru_var and is_break_var. If
is_break_tmp was set, we simply clear is_fallthru_tmp right after all
case tests to false.
Note that this approach does require maintaining a loop/switch stack as
the code originally did so that we can tell if a break statement is
associate with a loop or with a switch stmt. Only if the top of the
stack indicates that switch is being processed will the above break
processing be invoked.
Looking at a translation of my canonical example:
switch (expr) {
case c0:
case c1:
stmt0;
case c2:
case c3:
stmt1;
break;
case c4:
default:
stmt2;
}
We can translated this into:
int test_val_tmp = expr;
bool is_fallthru_tmp = false;
bool is_break_tmp = false;
if (test_val_tmp == c0) {
is_fallthru_tmp = true;
}
if (test_val_tmp == c1) {
is_fallthru_tmp = true;
}
if (is_break_tmp) {
is_fallthru_tmp = false;
}
if (is_fallthru_tmp) {
stmt0;
}
if (test_val_tmp == c2) {
is_fallthru_tmp = true;
}
if (test_val_tmp == c3) {
is_fallthru_tmp = true;
}
if (is_break_tmp) {
is_fallthru_tmp = false;
}
if (is_fallthru_tmp) {
stmt1;
is_break_tmp = true; // break stmt
}
if (test_val_tmp == c4) {
is_fallthru_tmp = true;
}
is_fallthru_tmp = true; // default
if (is_break_tmp) {
is_fallthru_tmp = false;
}
if (is_fallthru_tmp) {
stmt2;
}
Comparing this to what we did previously, the things that change in this
code are:
1) no enclosing loop
2) create a bool is_break_tmp initialized to false
3) after all case labels and before each case statement list, clear
is_fallthru_tmp when is_break_tmp
4) set is_break_tmp for each break statement
That sounds very reasonable.
And maintain the loop/switch stack again as in the original code.
I'll get this cranked out on Monday and resubmit for review. If anyone
has concerns or questions about what I'm doing here, please let me know.
cheers, danm
-BEGIN PGP SIGNATURE-
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/
iEYEARECAAYFAk4I+DEACgkQX1gOwKyEAw9bugCgiAH72AG2WnDQ0pvQAxMHTShd
NSgAn13bahn00ZW5vTxmnic2tWWG2+DS
=93jZ
-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/2 v2] glsl: Don't choke when printing an anonymous function parameter
On 06/27/2011 02:25 PM, Ian Romanick wrote: From: Ian Romanickian.d.roman...@intel.com NOTE: This is a candidate for the 7.10 and 7.11 branches. Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=38584 Strong work! I hadn't thought about anonymous parameters in prototypes. For both patch 1 and 2: Reviewed-by: Kenneth Graunke kenn...@whitecape.org ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] linker: Reject shaders that use too many varyings
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On 06/23/2011 07:56 AM, Alex Deucher wrote: On Thu, Jun 23, 2011 at 10:38 AM, Roland Scheidegger srol...@vmware.com wrote: Am 23.06.2011 16:09, schrieb Jerome Glisse: On Wed, Jun 22, 2011 at 10:49 PM, Alex Deucher alexdeuc...@gmail.com wrote: On Wed, Jun 22, 2011 at 10:12 PM, Roland Scheidegger srol...@vmware.com wrote: Am 21.06.2011 20:59, schrieb Sven Arvidsson: This change broke a whole lot of stuff on r600g, for example Unigine Heaven: shader uses too many varying components (36 32) It looks like the r600g driver claims to only support 10 varyings, which the state tracker reduces to 8 (as it subtracts the supposedly included color varyings). At first sight I can't quite see why it's limited to 10, all r600 chips should be able to handle 32 (dx10 requirement) but of course the driver might not (mesa itself is limited to 16 it seems). If it worked just fine before that suggests it indeed works just fine with more... Someone more familiar with the driver should be able to tell if it's safe to increase the limit to 32 (the state tracker will cap it to 16). The hardware definitely supports 32. I'm not sure why it's currently set to 10; I don't see any limitations in the code off hand. Alex IIRC it's just cut paste from r300g it can be safely bump Ok Marek bumped it to 34. That seems to be lying too I don't think it could handle 32 generic inputs and 2 colors. But there's no way to really express that right now. In addition to the routed inputs, you can also enable a bunch of hw generated inputs, but I think both generated and routed inputs combined are limited to 32 input slots. Was there any resolution to this issue? I'd like to cherry pick the linker patch to the 7.10 branch, but I don't want to regress r600g. -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.11 (GNU/Linux) Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/ iEYEARECAAYFAk4JAF8ACgkQX1gOwKyEAw/uZgCgmfWOXlfVQqW29DEajJzILWNm 5wsAniwJv88TkY4IjtUGt6tQu9/SRK6C =y29u -END PGP SIGNATURE- ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 37470] OpenGL ES2 test, black screen and test crashes
https://bugs.freedesktop.org/show_bug.cgi?id=37470 --- Comment #2 from Ian Romanick i...@freedesktop.org 2011-06-27 15:20:00 PDT --- There's also a non-ES2 version of this package (glmark2). Does that also reproduce this issue? Can you try running in GDB and get a proper backtrace? The one provided by glibc is not all that useful since it doesn't have source locations. -- 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] linker: Reject shaders that use too many varyings
Am 28.06.2011 00:12, schrieb Ian Romanick: On 06/23/2011 07:56 AM, Alex Deucher wrote: On Thu, Jun 23, 2011 at 10:38 AM, Roland Scheidegger srol...@vmware.com wrote: Am 23.06.2011 16:09, schrieb Jerome Glisse: On Wed, Jun 22, 2011 at 10:49 PM, Alex Deucher alexdeuc...@gmail.com wrote: On Wed, Jun 22, 2011 at 10:12 PM, Roland Scheidegger srol...@vmware.com wrote: Am 21.06.2011 20:59, schrieb Sven Arvidsson: This change broke a whole lot of stuff on r600g, for example Unigine Heaven: shader uses too many varying components (36 32) It looks like the r600g driver claims to only support 10 varyings, which the state tracker reduces to 8 (as it subtracts the supposedly included color varyings). At first sight I can't quite see why it's limited to 10, all r600 chips should be able to handle 32 (dx10 requirement) but of course the driver might not (mesa itself is limited to 16 it seems). If it worked just fine before that suggests it indeed works just fine with more... Someone more familiar with the driver should be able to tell if it's safe to increase the limit to 32 (the state tracker will cap it to 16). The hardware definitely supports 32. I'm not sure why it's currently set to 10; I don't see any limitations in the code off hand. Alex IIRC it's just cut paste from r300g it can be safely bump Ok Marek bumped it to 34. That seems to be lying too I don't think it could handle 32 generic inputs and 2 colors. But there's no way to really express that right now. In addition to the routed inputs, you can also enable a bunch of hw generated inputs, but I think both generated and routed inputs combined are limited to 32 input slots. Was there any resolution to this issue? I'd like to cherry pick the linker patch to the 7.10 branch, but I don't want to regress r600g. Yes, 1e5cef96d184b00eb588b48ecd02386998077d82 bumped the limits. It wasn't pushed to 7.10 though I believe it should be safe to do so. Roland ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 1/2] glsl: Changed extension enable bits to bools.
These were previously 1-bit-wide bitfields. Changing them to bools
has a negligible performance impact, and allows them to be accessed by
offset as well as by direct structure access.
---
src/glsl/glsl_parser_extras.h | 44
1 files changed, 22 insertions(+), 22 deletions(-)
diff --git a/src/glsl/glsl_parser_extras.h b/src/glsl/glsl_parser_extras.h
index 878d2ae..2f4d3cb 100644
--- a/src/glsl/glsl_parser_extras.h
+++ b/src/glsl/glsl_parser_extras.h
@@ -156,28 +156,28 @@ struct _mesa_glsl_parse_state {
* \name Enable bits for GLSL extensions
*/
/*@{*/
- unsigned ARB_draw_buffers_enable:1;
- unsigned ARB_draw_buffers_warn:1;
- unsigned ARB_draw_instanced_enable:1;
- unsigned ARB_draw_instanced_warn:1;
- unsigned ARB_explicit_attrib_location_enable:1;
- unsigned ARB_explicit_attrib_location_warn:1;
- unsigned ARB_fragment_coord_conventions_enable:1;
- unsigned ARB_fragment_coord_conventions_warn:1;
- unsigned ARB_texture_rectangle_enable:1;
- unsigned ARB_texture_rectangle_warn:1;
- unsigned EXT_texture_array_enable:1;
- unsigned EXT_texture_array_warn:1;
- unsigned ARB_shader_texture_lod_enable:1;
- unsigned ARB_shader_texture_lod_warn:1;
- unsigned ARB_shader_stencil_export_enable:1;
- unsigned ARB_shader_stencil_export_warn:1;
- unsigned AMD_conservative_depth_enable:1;
- unsigned AMD_conservative_depth_warn:1;
- unsigned AMD_shader_stencil_export_enable:1;
- unsigned AMD_shader_stencil_export_warn:1;
- unsigned OES_texture_3D_enable:1;
- unsigned OES_texture_3D_warn:1;
+ bool ARB_draw_buffers_enable;
+ bool ARB_draw_buffers_warn;
+ bool ARB_draw_instanced_enable;
+ bool ARB_draw_instanced_warn;
+ bool ARB_explicit_attrib_location_enable;
+ bool ARB_explicit_attrib_location_warn;
+ bool ARB_fragment_coord_conventions_enable;
+ bool ARB_fragment_coord_conventions_warn;
+ bool ARB_texture_rectangle_enable;
+ bool ARB_texture_rectangle_warn;
+ bool EXT_texture_array_enable;
+ bool EXT_texture_array_warn;
+ bool ARB_shader_texture_lod_enable;
+ bool ARB_shader_texture_lod_warn;
+ bool ARB_shader_stencil_export_enable;
+ bool ARB_shader_stencil_export_warn;
+ bool AMD_conservative_depth_enable;
+ bool AMD_conservative_depth_warn;
+ bool AMD_shader_stencil_export_enable;
+ bool AMD_shader_stencil_export_warn;
+ bool OES_texture_3D_enable;
+ bool OES_texture_3D_warn;
/*@}*/
/** Extensions supported by the OpenGL implementation. */
--
1.7.5.4
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 2/2] glsl: Rewrote _mesa_glsl_process_extension to use table-driven logic.
Instead of using a chain of manually maintained if/else blocks to
handle #extension directives, we now consult a table that specifies,
for each extension, the circumstances under which it is available, and
what flags in _mesa_glsl_parse_state need to be set in order to
activate it.
This makes it easier to add new GLSL extensions in the future, and
fixes the following bugs:
- Previously, _mesa_glsl_process_extension would sometimes set the
_enable and _warn flags for an extension before checking whether
the extension was supported by the driver; as a result, specifying
enable behavior for an unsupported extension would sometimes cause
front-end support for that extension to be switched on in spite of
the fact that back-end support was not available, leading to strange
failures, such as those in
https://bugs.freedesktop.org/show_bug.cgi?id=38015.
- #extension all: warn and #extension all: disable had no effect.
Notes:
- All extensions are currently marked as unavailable in geometry
shaders. This should not have any adverse effects since geometry
shaders aren't supported yet. When we return to working on geometry
shader support, we'll need to update the table for those extensions
that are avaiable in geometry shaders.
- Previous to this commit, if a shader mentioned
ARB_shader_texture_lod, extension ARB_texture_rectangle would be
automatically turned on in order to ensure that the types
sampler2DRect and sampler2DRectShadow would be defined. This was
unnecessary, because (a) ARB_shader_texture_lod works perfectly well
without those types provided that the builtin functions that
reference them are not called, and (b) ARB_texture_rectangle is
enabled by default in non-ES contexts anyway. I eliminated this
unnecessary behavior in order to make the behavior of all extensions
consistent.
---
src/glsl/glsl_parser_extras.cpp | 281 ---
1 files changed, 172 insertions(+), 109 deletions(-)
diff --git a/src/glsl/glsl_parser_extras.cpp b/src/glsl/glsl_parser_extras.cpp
index d9aa300..cdce231 100644
--- a/src/glsl/glsl_parser_extras.cpp
+++ b/src/glsl/glsl_parser_extras.cpp
@@ -165,133 +165,196 @@ _mesa_glsl_warning(const YYLTYPE *locp,
_mesa_glsl_parse_state *state,
}
+/**
+ * Enum representing the possible behaviors that can be specified in
+ * an #extension directive.
+ */
+enum ext_behavior {
+ extension_disable,
+ extension_enable,
+ extension_require,
+ extension_warn
+};
+
+/**
+ * Element type for _mesa_glsl_supported_extensions
+ */
+struct _mesa_glsl_extension {
+ /* Name of the extension when referred to in a GLSL extension
+* statement */
+ const char *name;
+
+ /* True if this extension is available to vertex shaders */
+ bool avail_in_VS;
+
+ /* True if this extension is available to geometry shaders */
+ bool avail_in_GS;
+
+ /* True if this extension is available to fragment shaders */
+ bool avail_in_FS;
+
+ /* True if this extension is available to desktop GL shaders */
+ bool avail_in_GL;
+
+ /* True if this extension is available to GLES shaders */
+ bool avail_in_ES;
+
+ /* Flag in the gl_extensions struct indicating whether this
+* extension is supported by the driver, or
+* gl_extensions::dummy_true if supported by all drivers */
+ const GLboolean gl_extensions::* supported_flag;
+
+ /* Flag in the _mesa_glsl_parse_state struct that should be set
+* when this extension is enabled. */
+ bool _mesa_glsl_parse_state::* enable_flag;
+
+ /* Flag in the _mesa_glsl_parse_state struct that should be set
+* when the shader requests warn behavior for this extension. */
+ bool _mesa_glsl_parse_state::* warn_flag;
+
+
+ bool compatible_with_state(const _mesa_glsl_parse_state *state) const;
+ void set_flags(_mesa_glsl_parse_state *state, ext_behavior behavior) const;
+};
+
+#define EXT(NAME, VS, GS, FS, GL, ES, SUPPORTED_FLAG) \
+ { GL_ #NAME, VS, GS, FS, GL, ES, gl_extensions::SUPPORTED_FLAG, \
+ _mesa_glsl_parse_state::NAME##_enable,\
+ _mesa_glsl_parse_state::NAME##_warn }
+
+/**
+ * Table of extensions that can be enabled/disabled within a shader,
+ * and the conditions under which they are supported.
+ */
+const _mesa_glsl_extension _mesa_glsl_supported_extensions[] = {
+ /* target availability API availability */
+ /* name VS GS FS GL ES
supported flag */
+ EXT(ARB_draw_buffers, false, false, true, true, false,
dummy_true),
+ EXT(ARB_draw_instanced, true, false, false, true, false,
ARB_draw_instanced),
+ EXT(ARB_explicit_attrib_location, true, false, true, true, false,
ARB_explicit_attrib_location),
+ EXT(ARB_fragment_coord_conventions, true, false, true, true, false,
ARB_fragment_coord_conventions),
+
[Mesa-dev] [PATCH] glsl: Track killed mask in constant propagation live set
From: Ian Romanick ian.d.roman...@intel.com
The kill mask and the initial assignment mask must be tracked with
each constant in the set. Otherwise the wrong component can be
selected after earlier components have been killed.
NOTE: This is a candidate for the 7.10 and 7.11 branches.
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=37383
Cc: Eric Anholt e...@anholt.net
Cc: Kenneth Graunke kenn...@whitecape.org
Cc: Matthias Bentrup matthias.bent...@googlemail.com
---
src/glsl/opt_constant_propagation.cpp |6 +-
1 files changed, 5 insertions(+), 1 deletions(-)
diff --git a/src/glsl/opt_constant_propagation.cpp
b/src/glsl/opt_constant_propagation.cpp
index 4425f42..414f0f2 100644
--- a/src/glsl/opt_constant_propagation.cpp
+++ b/src/glsl/opt_constant_propagation.cpp
@@ -51,11 +51,13 @@ public:
this-var = var;
this-write_mask = write_mask;
this-constant = constant;
+ this-kill_mask = 0;
}
ir_variable *var;
ir_constant *constant;
unsigned write_mask;
+ unsigned kill_mask;
};
@@ -169,10 +171,11 @@ ir_constant_propagation_visitor::handle_rvalue(ir_rvalue
**rvalue)
return;
int rhs_channel = 0;
+ const unsigned mask = found-write_mask | found-kill_mask;
for (int j = 0; j 4; j++) {
if (j == channel)
break;
-if (found-write_mask (1 j))
+if (mask (1 j))
rhs_channel++;
}
@@ -369,6 +372,7 @@ ir_constant_propagation_visitor::kill(ir_variable *var,
unsigned write_mask)
if (entry-var == var) {
entry-write_mask = ~write_mask;
+entry-kill_mask |= write_mask;
if (entry-write_mask == 0)
entry-remove();
}
--
1.7.4.4
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 6/6] glsl: Generate IR for switch statements
Here's another case that I'm not sure you're handling
correctly...conditional breaks:
switch (expr) {
case c0:
case c1:
stmt0;
case c2:
case c3:
stmt1;
break;
case c4:
stmt2;
if (foo)
break;
stmt3;// happens if !foo
case c5:
default:
stmt4;// happens if (expr != c4 || foo)
};
The easiest solution I can think of is to (in C++) track whether a break
-might- have been taken and emit a guard around further statements:
stmt2;
if (foo)
has_break_tmp = true; // hit the break statement
if (has_break_tmp) { // could have potentially hit break, must check
stmt3;
}
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 6/6] glsl: Generate IR for switch statements
On 06/24/2011 05:11 PM, Dan McCabe wrote:
Looking at a translation of my canonical example:
switch (expr) {
case c0:
case c1:
stmt0;
case c2:
case c3:
stmt1;
break;
case c4:
default:
stmt2;
}
We can translated this into:
int test_val_tmp = expr;
bool is_fallthru_tmp = false;
bool is_break_tmp = false;
if (test_val_tmp == c0) {
is_fallthru_tmp = true;
}
if (test_val_tmp == c1) {
is_fallthru_tmp = true;
}
if (is_break_tmp) {
is_fallthru_tmp = false;
}
if (is_fallthru_tmp) {
stmt0;
}
if (test_val_tmp == c2) {
is_fallthru_tmp = true;
}
if (test_val_tmp == c3) {
is_fallthru_tmp = true;
}
if (is_break_tmp) {
is_fallthru_tmp = false;
}
if (is_fallthru_tmp) {
stmt1;
is_break_tmp = true; // break stmt
}
if (test_val_tmp == c4) {
is_fallthru_tmp = true;
}
is_fallthru_tmp = true; // default
if (is_break_tmp) {
is_fallthru_tmp = false;
}
if (is_fallthru_tmp) {
stmt2;
}
I must admit, I'm not particularly crazy about the amount of
if-statements being generated here. A couple of ideas...
1. Use conditional moves:
(assign (ir_expression all_equal (var_ref test_val_tmp) c0) ; condition
(var_ref is_fallthru_tmp) ; assignee
(constant bool (1))) ; true
Then again Ian's new optimization pass should do this for us, so maybe
leaving it as if-statements is fine.
Since we'd have two conditional moves (for c0 and c1) with the same
target and value, it'd be nice to combine them...but that sounds like a
pretty obscure optimization pass.
2. Use ORs:
is_fallthru_tmp = false;
is_fallthru_tmp = is_fallthru_tmp || test_val_tmp == c0;
is_fallthru_tmp = is_fallthru_tmp || test_val_tmp == c1;
This seems somewhat easier to optimize...I imagine tree-grafting,
constant/copy propagation, etc. might be able to reduce this to:
is_fallthru_tmp = test_val_tmp == c0 || test_val_tmp == c1;
Again, I'm not sure. Ian, do you have an opinion?
In the end, this may be a moot point---we eventually want to take
advantage of hardware jump instructions. When switching on a uniform
(I'm guessing this is common), all the channels would take the same
path, so a simple ADD to the IP would work. Even for non-uniform
control flow, I suspect we could use the Gen break instruction, which
will jump if all channels take the path. But that's for later.
--Kenneth
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 6/6] glsl: Generate IR for switch statements
On 06/27/2011 05:15 PM, Kenneth Graunke wrote:
On 06/24/2011 05:11 PM, Dan McCabe wrote:
Looking at a translation of my canonical example:
switch (expr) {
case c0:
case c1:
stmt0;
case c2:
case c3:
stmt1;
break;
case c4:
default:
stmt2;
}
We can translated this into:
int test_val_tmp = expr;
bool is_fallthru_tmp = false;
bool is_break_tmp = false;
if (test_val_tmp == c0) {
is_fallthru_tmp = true;
}
if (test_val_tmp == c1) {
is_fallthru_tmp = true;
}
if (is_break_tmp) {
is_fallthru_tmp = false;
}
if (is_fallthru_tmp) {
stmt0;
}
if (test_val_tmp == c2) {
is_fallthru_tmp = true;
}
if (test_val_tmp == c3) {
is_fallthru_tmp = true;
}
if (is_break_tmp) {
is_fallthru_tmp = false;
}
if (is_fallthru_tmp) {
stmt1;
is_break_tmp = true; // break stmt
}
if (test_val_tmp == c4) {
is_fallthru_tmp = true;
}
is_fallthru_tmp = true; // default
if (is_break_tmp) {
is_fallthru_tmp = false;
}
if (is_fallthru_tmp) {
stmt2;
}
I must admit, I'm not particularly crazy about the amount of
if-statements being generated here. A couple of ideas...
1. Use conditional moves:
(assign (ir_expression all_equal (var_ref test_val_tmp) c0) ; condition
(var_ref is_fallthru_tmp) ; assignee
(constant bool (1))) ; true
Then again Ian's new optimization pass should do this for us, so maybe
leaving it as if-statements is fine.
Since we'd have two conditional moves (for c0 and c1) with the same
target and value, it'd be nice to combine them...but that sounds like
a pretty obscure optimization pass.
2. Use ORs:
is_fallthru_tmp = false;
is_fallthru_tmp = is_fallthru_tmp || test_val_tmp == c0;
is_fallthru_tmp = is_fallthru_tmp || test_val_tmp == c1;
This seems somewhat easier to optimize...I imagine tree-grafting,
constant/copy propagation, etc. might be able to reduce this to:
is_fallthru_tmp = test_val_tmp == c0 || test_val_tmp == c1;
Again, I'm not sure. Ian, do you have an opinion?
In the end, this may be a moot point---we eventually want to take
advantage of hardware jump instructions. When switching on a uniform
(I'm guessing this is common), all the channels would take the same
path, so a simple ADD to the IP would work. Even for non-uniform
control flow, I suspect we could use the Gen break instruction,
which will jump if all channels take the path. But that's for later.
--Kenneth
Since I am just about to start modifying the IR generation, I'm open to
suggestion.
#1 looks like a better choice. But I might change my mind when I
actually get to modifying the code.
I originally considered #2, but the IR that was generated by such code
didn't look any better (i.e., you would up with a bunch of IF code
anyway). That is why I went the route I did.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH-RFC] autoconf: add --enable-{dri,glx,osmesa}
On Tue, Jun 28, 2011 at 4:17 AM, Dan Nicholson dbn.li...@gmail.com wrote:
On Sun, Jun 26, 2011 at 6:39 AM, Chia-I Wu olva...@gmail.com wrote:
From: Chia-I Wu o...@lunarg.com
The idea is that DRI driver, libGL and libOSMesa are libraries that can
be independently enabled, yet --with-driver does not allow us to easily
do that, if not impossible. This also matches what
--enable-{egl,xorg,d3d1x} do for the respective libraries.
I haven't read this in any detail, but I definitely like the idea.
When I originally wrote all this, I struggled to coordinate DRI vs.
GLX, and I didn't really bother with the EGL code that was mostly
experimental. This is much more coherently structured.
Great.
There are two libGL providers: Xlib-based and DRI-based. They cannot
coexist. To be able to choose between them, --enable-xlib-glx is also
added.
This is the only part that kind of bugs me. It seems to me that the
--enable-dri and --enable-xlib-glx options aren't really symmetric. I
believe you'd need this to be --enable-dri-glx to really act as a
provider. I can see why you didn't do that since dri is a provider
to many of the APIs and would require a lot more hacking of
configure.ac. Is my understanding of that correct? I'm not as familiar
with the newer non-GL Mesa components.
I would like APIs to be enabled in the most straightforward ways.
Say, when I need GLX and D3D1X, I should be able to express that by
specifying
--enable-glx --enable-d3d1x
(--enable-glx is redundant as it is default on)
These API options are the high-level ones.
Then there are lower levels of options to change how APIs are
implemented. Users usually don't need to care about these options as
we pick the best we have by default (finger crossed). Examples of
such options are, for GLX,
--enable-xcb Use XCB in DRI-based GLX
--enable-driglx-direct Support direct rendering in DRI-based GLX
(here DRI-based actually means {GLX/DRI2}PROTO-based)
--enable-xlib-glx Use Xlib-based GLX instead
(and the first two options become irrelevant)
For DRI, there is --enable-shared-dricore; for EGL, there is
--enable-gallium-egl. And so on.
This is the idea behind those options. I hope this expresses my idea
more clear, and help us come up with a more systematical way to name
the options.
--
Dan
--
o...@lunarg.com
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 6/6] glsl: Generate IR for switch statements
On Mon, 27 Jun 2011 17:23:30 -0700, Dan McCabe zen3d.li...@gmail.com wrote: Since I am just about to start modifying the IR generation, I'm open to suggestion. I'd write the simplest possible code that works now and then consider optimizing it later. This has the advantage that you can get a bunch of tests working and then use those to validate a later, more complicated, implementation. -- keith.pack...@intel.com pgp0t5A5C5P2p.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] glsl: Generate IR for switch statements
On 06/27/2011 04:40 PM, Kenneth Graunke wrote:
Here's another case that I'm not sure you're handling
correctly...conditional breaks:
switch (expr) {
case c0:
case c1:
stmt0;
case c2:
case c3:
stmt1;
break;
case c4:
stmt2;
if (foo)
break;
stmt3;// happens if !foo
case c5:
default:
stmt4;// happens if (expr != c4 || foo)
};
The easiest solution I can think of is to (in C++) track whether a
break -might- have been taken and emit a guard around further statements:
stmt2;
if (foo)
has_break_tmp = true; // hit the break statement
if (has_break_tmp) { // could have potentially hit break, must check
stmt3;
}
This is similar to the proposal that I floated Fri PM and that you
commented upon at 17:15 today.
Please correct me if I'm wrong.
cheers, danm
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 6/6] glsl: Generate IR for switch statements
On 06/27/2011 05:35 PM, Keith Packard wrote: On Mon, 27 Jun 2011 17:23:30 -0700, Dan McCabezen3d.li...@gmail.com wrote: Since I am just about to start modifying the IR generation, I'm open to suggestion. I'd write the simplest possible code that works now and then consider optimizing it later. This has the advantage that you can get a bunch of tests working and then use those to validate a later, more complicated, implementation. Arguably Kenneth's suggestion to use conditional assignment is simpler. But it does entail some mods to what I already wrote. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH-RFC] autoconf: add --enable-{dri,glx,osmesa}
On Tue, Jun 28, 2011 at 6:00 AM, Kenneth Graunke kenn...@whitecape.org wrote:
On 06/27/2011 12:17 PM, Dan Nicholson wrote:
On Sun, Jun 26, 2011 at 6:39 AM, Chia-I Wuolva...@gmail.com wrote:
From: Chia-I Wuo...@lunarg.com
The idea is that DRI driver, libGL and libOSMesa are libraries that can
be independently enabled, yet --with-driver does not allow us to easily
do that, if not impossible. This also matches what
--enable-{egl,xorg,d3d1x} do for the respective libraries.
I haven't read this in any detail, but I definitely like the idea.
When I originally wrote all this, I struggled to coordinate DRI vs.
GLX, and I didn't really bother with the EGL code that was mostly
experimental. This is much more coherently structured.
There are two libGL providers: Xlib-based and DRI-based. They cannot
coexist. To be able to choose between them, --enable-xlib-glx is also
added.
This is the only part that kind of bugs me. It seems to me that the
--enable-dri and --enable-xlib-glx options aren't really symmetric. I
believe you'd need this to be --enable-dri-glx to really act as a
provider. I can see why you didn't do that since dri is a provider
to many of the APIs and would require a lot more hacking of
configure.ac. Is my understanding of that correct? I'm not as familiar
with the newer non-GL Mesa components.
How does --enable-xcb (use XCB for GLX) fit in? I've been using that for
ages. Why isn't it the default these days? XCB isn't exactly cutting edge,
and I believe we already use it for some DRI2 stuff. Is there some
Xlib-based stuff that still needs to be converted? Can we just kill
Xlib-GLX entirely?
Xlib-based GLX (sources under $mesa/src/mesa/drivers/glx) is the faked
GLX, emulated with only core protocol. I was not clear. I just
thought that fake GLX sounded scary.
So --enable-xcb enables the use of XCB in DRI-based GLX
($mesa/src/glx). It is relevant when GLX is enabled and DRI-based GLX
is chosen.
I'm probably not understanding any of this...sorry :)
--Kenneth
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
--
o...@lunarg.com
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 2/2] glsl: Rewrote _mesa_glsl_process_extension to use table-driven logic.
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1
I like this a lot. It's a really good clean up of a rotting cesspool.
I have a few formatting / style comments below and a question.
On 06/27/2011 04:26 PM, Paul Berry wrote:
Instead of using a chain of manually maintained if/else blocks to
handle #extension directives, we now consult a table that specifies,
for each extension, the circumstances under which it is available, and
what flags in _mesa_glsl_parse_state need to be set in order to
activate it.
This makes it easier to add new GLSL extensions in the future, and
fixes the following bugs:
- Previously, _mesa_glsl_process_extension would sometimes set the
_enable and _warn flags for an extension before checking whether
the extension was supported by the driver; as a result, specifying
enable behavior for an unsupported extension would sometimes cause
front-end support for that extension to be switched on in spite of
the fact that back-end support was not available, leading to strange
failures, such as those in
https://bugs.freedesktop.org/show_bug.cgi?id=38015.
- #extension all: warn and #extension all: disable had no effect.
Notes:
- All extensions are currently marked as unavailable in geometry
shaders. This should not have any adverse effects since geometry
shaders aren't supported yet. When we return to working on geometry
shader support, we'll need to update the table for those extensions
that are avaiable in geometry shaders.
available
- Previous to this commit, if a shader mentioned
ARB_shader_texture_lod, extension ARB_texture_rectangle would be
automatically turned on in order to ensure that the types
sampler2DRect and sampler2DRectShadow would be defined. This was
unnecessary, because (a) ARB_shader_texture_lod works perfectly well
without those types provided that the builtin functions that
reference them are not called, and (b) ARB_texture_rectangle is
enabled by default in non-ES contexts anyway. I eliminated this
unnecessary behavior in order to make the behavior of all extensions
consistent.
---
src/glsl/glsl_parser_extras.cpp | 281
---
1 files changed, 172 insertions(+), 109 deletions(-)
diff --git a/src/glsl/glsl_parser_extras.cpp b/src/glsl/glsl_parser_extras.cpp
index d9aa300..cdce231 100644
--- a/src/glsl/glsl_parser_extras.cpp
+++ b/src/glsl/glsl_parser_extras.cpp
@@ -165,133 +165,196 @@ _mesa_glsl_warning(const YYLTYPE *locp,
_mesa_glsl_parse_state *state,
}
+/**
+ * Enum representing the possible behaviors that can be specified in
+ * an #extension directive.
+ */
+enum ext_behavior {
+ extension_disable,
+ extension_enable,
+ extension_require,
+ extension_warn
+};
+
+/**
+ * Element type for _mesa_glsl_supported_extensions
+ */
+struct _mesa_glsl_extension {
+ /* Name of the extension when referred to in a GLSL extension
+* statement */
Field comments should get the doxygen
/**
* Short description
*
* Detailed description
*/
or
/** Description */
treatment.
Also, we generally prefer the closing */ of a multiline comment to be on
its own line.
+ const char *name;
+
+ /* True if this extension is available to vertex shaders */
+ bool avail_in_VS;
+
+ /* True if this extension is available to geometry shaders */
+ bool avail_in_GS;
+
+ /* True if this extension is available to fragment shaders */
+ bool avail_in_FS;
+
+ /* True if this extension is available to desktop GL shaders */
+ bool avail_in_GL;
+
+ /* True if this extension is available to GLES shaders */
+ bool avail_in_ES;
+
+ /* Flag in the gl_extensions struct indicating whether this
+* extension is supported by the driver, or
+* gl_extensions::dummy_true if supported by all drivers */
+ const GLboolean gl_extensions::* supported_flag;
WTF? Seriously. What does this do? I was expecting to see this code
use the offsetof macro (like src/mesa/main/extensions.c does), but I'm
now suspecting that C++ has some built-in magic for this. Is that true?
+
+ /* Flag in the _mesa_glsl_parse_state struct that should be set
+* when this extension is enabled. */
+ bool _mesa_glsl_parse_state::* enable_flag;
+
+ /* Flag in the _mesa_glsl_parse_state struct that should be set
+* when the shader requests warn behavior for this extension. */
+ bool _mesa_glsl_parse_state::* warn_flag;
+
+
+ bool compatible_with_state(const _mesa_glsl_parse_state *state) const;
+ void set_flags(_mesa_glsl_parse_state *state, ext_behavior behavior)
const;
+};
+
+#define EXT(NAME, VS, GS, FS, GL, ES, SUPPORTED_FLAG) \
+ { GL_ #NAME, VS, GS, FS, GL, ES, gl_extensions::SUPPORTED_FLAG, \
+ _mesa_glsl_parse_state::NAME##_enable,\
+ _mesa_glsl_parse_state::NAME##_warn }
+
[Mesa-dev] [Bug 37470] OpenGL ES2 test, black screen and test crashes
https://bugs.freedesktop.org/show_bug.cgi?id=37470 --- Comment #3 from apfelmausm...@googlemail.com 2011-06-27 18:31:53 PDT --- hm after retest it.. now the -es2 one works fine with more frames then the normal opengl one. I think its fixed for me -- 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
[Mesa-dev] [PATCH v2] glsl: Track initial mask in constant propagation live set
From: Ian Romanick ian.d.roman...@intel.com
The set of values initially available (before any kills) must be
tracked with each constant in the set. Otherwise the wrong component
can be selected after earlier components have been killed.
NOTE: This is a candidate for the 7.10 and 7.11 branches.
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=37383
Cc: Eric Anholt e...@anholt.net
Cc: Kenneth Graunke kenn...@whitecape.org
Cc: Matthias Bentrup matthias.bent...@googlemail.com
---
v2 fixes some regressions in the previous version. Instead of
tracking the kill mask in the ACP, the initial value mask is tracked.
A copy constructor is added to make sure the correct initial mask is
propagated to copies.
src/glsl/opt_constant_propagation.cpp | 17 ++---
1 files changed, 14 insertions(+), 3 deletions(-)
diff --git a/src/glsl/opt_constant_propagation.cpp
b/src/glsl/opt_constant_propagation.cpp
index 4425f42..af77e49 100644
--- a/src/glsl/opt_constant_propagation.cpp
+++ b/src/glsl/opt_constant_propagation.cpp
@@ -51,11 +51,23 @@ public:
this-var = var;
this-write_mask = write_mask;
this-constant = constant;
+ this-initial_values = write_mask;
+ }
+
+ acp_entry(const acp_entry *src)
+ {
+ this-var = src-var;
+ this-write_mask = src-write_mask;
+ this-constant = src-constant;
+ this-initial_values = src-initial_values;
}
ir_variable *var;
ir_constant *constant;
unsigned write_mask;
+
+ /** Mask of values initially available in the constant. */
+ unsigned initial_values;
};
@@ -172,7 +184,7 @@ ir_constant_propagation_visitor::handle_rvalue(ir_rvalue
**rvalue)
for (int j = 0; j 4; j++) {
if (j == channel)
break;
-if (found-write_mask (1 j))
+if (found-initial_values (1 j))
rhs_channel++;
}
@@ -285,8 +297,7 @@ ir_constant_propagation_visitor::handle_if_block(exec_list
*instructions)
/* Populate the initial acp with a constant of the original */
foreach_iter(exec_list_iterator, iter, *orig_acp) {
acp_entry *a = (acp_entry *)iter.get();
- this-acp-push_tail(new(this-mem_ctx) acp_entry(a-var, a-write_mask,
- a-constant));
+ this-acp-push_tail(new(this-mem_ctx) acp_entry(a));
}
visit_list_elements(this, instructions);
--
1.7.4.4
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
