[Mesa-dev] [PATCH 1/2] Allow for GLSL shaders to have #extension directive anywhere in source file.

[Kevin Rogovin]

Allow for GLSL shaders to have #extension directive
anywhere in source file; during preprocessor, #extension
directives are saved to a list rather than being added
to the pre-processed string. That list is played back 
before parsing of pre-processed text takes place.  

---
 src/glsl/Makefile.sources   |   3 +-
 src/glsl/glcpp/glcpp-lex.l  |  19 +-
 src/glsl/glcpp/glcpp-parse.y|  31 +-
 src/glsl/glcpp/glcpp.c  |   2 +-
 src/glsl/glcpp/glcpp.h  |  17 +-
 src/glsl/glcpp/glcpp_extras.c   | 126 
 src/glsl/glcpp/glcpp_extras.h   |  97 +++
 src/glsl/glcpp/pp.c |   5 +-
 src/glsl/glsl_parser_extras.cpp |  55 +-
 src/glsl/glsl_parser_extras.h   |  14 -
 10 files changed, 358 insertions(+), 11 deletions(-)
 create mode 100644 src/glsl/glcpp/glcpp_extras.c
 create mode 100644 src/glsl/glcpp/glcpp_extras.h

diff --git a/src/glsl/Makefile.sources b/src/glsl/Makefile.sources
index 2e81ded..90f57ad 100644
--- a/src/glsl/Makefile.sources
+++ b/src/glsl/Makefile.sources
@@ -7,7 +7,8 @@ GLSL_BUILDDIR = $(top_builddir)/src/glsl
 
 LIBGLCPP_FILES = \
$(GLSL_SRCDIR)/ralloc.c \
-   $(GLSL_SRCDIR)/glcpp/pp.c
+   $(GLSL_SRCDIR)/glcpp/pp.c \
+$(GLSL_SRCDIR)/glcpp/glcpp_extras.c
 
 LIBGLCPP_GENERATED_FILES = \
$(GLSL_BUILDDIR)/glcpp/glcpp-lex.c \
diff --git a/src/glsl/glcpp/glcpp-lex.l b/src/glsl/glcpp/glcpp-lex.l
index a029f62..31b246f 100644
--- a/src/glsl/glcpp/glcpp-lex.l
+++ b/src/glsl/glcpp/glcpp-lex.l
@@ -126,15 +126,30 @@ HEXADECIMAL_INTEGER   0[xX][0-9a-fA-F]+[uU]?
return HASH_VERSION;
 }
 
-   /* glcpp doesn't handle #extension, #version, or #pragma directives.
+   /* glcpp doesn't handle #pragma directives.
 * Simply pass them through to the main compiler's lexer/parser. */
-{HASH}(extension|pragma)[^\n]+ {
+{HASH}(pragma)[^\n]+ {
yylval->str = ralloc_strdup (yyextra, yytext);
yylineno++;
yycolumn = 0;
return OTHER;
 }
 
+/* glcpp will handle the #extension directive if
+* and only if parser->directive_ext_list is non-NULL
+* otherwise it will just send that off to the parser */
+{HASH}(extension)[^\n]+ {
+yylval->str = ralloc_strdup (yyextra, yytext);
+   yylineno++;
+   yycolumn = 0;
+   if(parser->directive_ext_list) {
+ return EXTENSION_DIRECTIVE;
+} else {
+ return OTHER;
+   }
+}
+
+
 {HASH}line{HSPACE}+ {
return HASH_LINE;
 }
diff --git a/src/glsl/glcpp/glcpp-parse.y b/src/glsl/glcpp/glcpp-parse.y
index 7edc274..d1d58c3 100644
--- a/src/glsl/glcpp/glcpp-parse.y
+++ b/src/glsl/glcpp/glcpp-parse.y
@@ -166,6 +166,7 @@ add_builtin_define(glcpp_parser_t *parser, const char 
*name, int value);
 %expect 0
 %token COMMA_FINAL DEFINED ELIF_EXPANDED HASH HASH_DEFINE FUNC_IDENTIFIER 
OBJ_IDENTIFIER HASH_ELIF HASH_ELSE HASH_ENDIF HASH_IF HASH_IFDEF HASH_IFNDEF 
HASH_LINE HASH_UNDEF HASH_VERSION IDENTIFIER IF_EXPANDED INTEGER INTEGER_STRING 
LINE_EXPANDED NEWLINE OTHER PLACEHOLDER SPACE
 %token PASTE
+%token  EXTENSION_DIRECTIVE
 %type  expression INTEGER operator SPACE integer_constant
 %type  IDENTIFIER FUNC_IDENTIFIER OBJ_IDENTIFIER INTEGER_STRING OTHER
 %type  identifier_list
@@ -208,9 +209,34 @@ line:
ralloc_free ($1);
}
 |  expanded_line
+|   extension_directive
 |  HASH non_directive
 ;
 
+extension_directive:
+EXTENSION_DIRECTIVE {
+   int cnt;
+   
+   assert(parser->directive_ext_list);
+   /*
+ the lexer increments yylineno when it encounters
+ #extension. If it did not, then the line used by
+ the glsl parser when then be wrong. We get around
+ the issue by decrementing by number of #extension
+ processed so far.
+*/
+   cnt=parser->directive_ext_list->count;
+   add_extension_from_preprocessor($1, 
+   @1.first_line - cnt, 
+   @1.first_column,
+   @1.last_line - cnt,
+   @1.last_column,
+   @1.source,
+   parser->directive_ext_list); 
+}
+;
+
+
 expanded_line:
IF_EXPANDED expression NEWLINE {
_glcpp_parser_skip_stack_push_if (parser, & @1, $2);
@@ -1148,7 +1174,8 @@ static void add_builtin_define(glcpp_parser_t *parser,
 }
 
 glcpp_parser_t *
-glcpp_parser_create (const struct gl_extensions *extensions, int api)
+glcpp_parser_create (const struct gl_extensions *extensions, int api,
+ struct glcpp_extension_directive_list *ext_list)
 {
glcpp_parser_t *parser;
int language_version;
@@ -1182,6 +1209,7 @@ glcpp_parser_create (const struct gl_

[Mesa-dev] [PATCH 2/2] add MESA_GLSL option to require that #extension directives come before any variable or function declarations, etc

[Kevin Rogovin]

Add an additional bit flag, GLSL_EXTENSION_STRICT for 
gl_context#Shader#Flags to require that #extension 
directives must come before variable of function
declarations. The flag is set to on if MESA_GLSL has
the option extension_strict_ordering in it.

---
 src/glsl/glsl_parser_extras.cpp | 6 +++---
 src/mesa/main/mtypes.h  | 8 +++-
 src/mesa/main/shaderapi.c   | 2 ++
 3 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/src/glsl/glsl_parser_extras.cpp b/src/glsl/glsl_parser_extras.cpp
index 75640e1..25d16ab 100644
--- a/src/glsl/glsl_parser_extras.cpp
+++ b/src/glsl/glsl_parser_extras.cpp
@@ -1515,11 +1515,11 @@ _mesa_glsl_compile_shader(struct gl_context *ctx, 
struct gl_shader *shader,
   new(shader) _mesa_glsl_parse_state(ctx, shader->Type, shader);
const char *source = shader->Source;
 
-   if(1) {
+   if(ctx->Shader.Flags&GLSL_EXTENSION_STRICT) {
+  state->ext_directive_list=NULL;
+   } else {
   state->ext_directive_list=&extension_directive_list;
   init_glcpp_extension_directive_list(state->ext_directive_list);
-   } else {
-  state->ext_directive_list=NULL;
}
 
state->error = glcpp_preprocess(state, &source, &state->info_log,
diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h
index b4b432f..28cb5f9 100644
--- a/src/mesa/main/mtypes.h
+++ b/src/mesa/main/mtypes.h
@@ -2652,7 +2652,13 @@ struct gl_shader_program
 #define GLSL_USE_PROG 0x80  /**< Log glUseProgram calls */
 #define GLSL_REPORT_ERRORS 0x100  /**< Print compilation errors */
 #define GLSL_DUMP_ON_ERROR 0x200 /**< Dump shaders to stderr on compile error 
*/
-
+/**
+ * In GLSL sources, follow specifications 
+ * with respect to #extension ordering, i.e.
+ * such directives must come before function
+ * declarations, variable declarations, etc.
+ */
+#define GLSL_EXTENSION_STRICT 0x400 
 
 /**
  * Context state for GLSL vertex/fragment shaders.
diff --git a/src/mesa/main/shaderapi.c b/src/mesa/main/shaderapi.c
index 1d9aac3..dd12f42 100644
--- a/src/mesa/main/shaderapi.c
+++ b/src/mesa/main/shaderapi.c
@@ -92,6 +92,8 @@ get_shader_flags(void)
  flags |= GLSL_USE_PROG;
   if (strstr(env, "errors"))
  flags |= GLSL_REPORT_ERRORS;
+  if(strstr(env, "extension_strict_ordering"))
+ flags |= GLSL_EXTENSION_STRICT;
}
 
return flags;
-- 
1.8.1.2

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

Re: [Mesa-dev] [PATCH 1/2] Allow for GLSL shaders to have #extension directive anywhere in source file.

[Kenneth Graunke]

On 12/03/2013 12:00 AM, Kevin Rogovin wrote:
> Allow for GLSL shaders to have #extension directive
> anywhere in source file; during preprocessor, #extension
> directives are saved to a list rather than being added
> to the pre-processed string. That list is played back 
> before parsing of pre-processed text takes place.  
> 
> ---
>  src/glsl/Makefile.sources   |   3 +-
>  src/glsl/glcpp/glcpp-lex.l  |  19 +-
>  src/glsl/glcpp/glcpp-parse.y|  31 +-
>  src/glsl/glcpp/glcpp.c  |   2 +-
>  src/glsl/glcpp/glcpp.h  |  17 +-
>  src/glsl/glcpp/glcpp_extras.c   | 126 
> 
>  src/glsl/glcpp/glcpp_extras.h   |  97 +++
>  src/glsl/glcpp/pp.c |   5 +-
>  src/glsl/glsl_parser_extras.cpp |  55 +-
>  src/glsl/glsl_parser_extras.h   |  14 -
>  10 files changed, 358 insertions(+), 11 deletions(-)
>  create mode 100644 src/glsl/glcpp/glcpp_extras.c
>  create mode 100644 src/glsl/glcpp/glcpp_extras.h

Hi Kevin,

This is a single patch that performs a ton of refactoring as well as a
functional change.  It would be great if you could break it down into
smaller parts.  For example, perhaps you could begin parsing and
tracking extension directives, but not change how they're actually
processed.  Then, you could change it to handle them in glcpp, but still
require them to come first.  Finally, you could change the behavior to
accept them anywhere.

Submitting smaller patches makes it easier to review them.  It also
makes bisecting more accurate, for tracking down any bugs.  Finally, it
means that if you have a contentious part that might require some
rework, you can get review and possibly land the rest of your series,
and not have to respin it.

A couple of other comments:

Please follow the style of the existing glcpp code.  Annoyingly, glcpp
currently uses Carl's style, with is different than the rest of Mesa.
However, it's internally very consistent and clean, and I'd prefer to
keep it that way.

If you use vim, you can accomplish the appropriate indent settings by
putting this in your .vimrc file:

if has("autocmd")
au BufNewFile,BufRead */mesa/src/glsl/glcpp/* set noexpandtab
tabstop=8 softtabstop=8 shiftwidth=8
au BufNewFile,BufRead */mesa/* set expandtab tabstop=8
softtabstop=3 shiftwidth=3
endif

(I'm sure there's appropriate magic for Emacs or other editors as well.)

Other than indentation, a couple of nits:

/* Normal multiline comments should have
 * stars at the beginning of lines, even in the middle
 */

/**
 * Doxygen should look like this.
 *
 * with the double star at the top and stars in the middle.
 */

Please put spaces around operators and between if/while/for and parens.
 For example:

   while(current!=NULL) {  /* wrong, vs. */
   while (current != NULL) {   /* correct */

   cnt=parser->directive_ext_list->count;   /* wrong */
   cnt = parser->directive_ext_list->count; /* correct */

   for(current=state->ext_directive_list->head; current!=NULL;
current=current->next) {   /* wrong */

Also, this is the first use of MALLOC, FREE, and so on in the compiler
code.  These are stupid #defines for malloc, free, and so on.  Please
just use the standard lowercase C library functions.

Until this patch series is split up and in a more consistent style, I
don't plan to review it further.

--Ken

> diff --git a/src/glsl/Makefile.sources b/src/glsl/Makefile.sources
> index 2e81ded..90f57ad 100644
> --- a/src/glsl/Makefile.sources
> +++ b/src/glsl/Makefile.sources
> @@ -7,7 +7,8 @@ GLSL_BUILDDIR = $(top_builddir)/src/glsl
>  
>  LIBGLCPP_FILES = \
>   $(GLSL_SRCDIR)/ralloc.c \
> - $(GLSL_SRCDIR)/glcpp/pp.c
> + $(GLSL_SRCDIR)/glcpp/pp.c \
> +$(GLSL_SRCDIR)/glcpp/glcpp_extras.c
>  
>  LIBGLCPP_GENERATED_FILES = \
>   $(GLSL_BUILDDIR)/glcpp/glcpp-lex.c \
> diff --git a/src/glsl/glcpp/glcpp-lex.l b/src/glsl/glcpp/glcpp-lex.l
> index a029f62..31b246f 100644
> --- a/src/glsl/glcpp/glcpp-lex.l
> +++ b/src/glsl/glcpp/glcpp-lex.l
> @@ -126,15 +126,30 @@ HEXADECIMAL_INTEGER 0[xX][0-9a-fA-F]+[uU]?
>   return HASH_VERSION;
>  }
>  
> - /* glcpp doesn't handle #extension, #version, or #pragma directives.
> + /* glcpp doesn't handle #pragma directives.
>* Simply pass them through to the main compiler's lexer/parser. */
> -{HASH}(extension|pragma)[^\n]+ {
> +{HASH}(pragma)[^\n]+ {
>   yylval->str = ralloc_strdup (yyextra, yytext);
>   yylineno++;
>   yycolumn = 0;
>   return OTHER;
>  }
>  
> +/* glcpp will handle the #extension directive if
> +  * and only if parser->directive_ext_list is non-NULL
> +  * otherwise it will just send that off to the parser */
> +{HASH}(extension)[^\n]+ {
> +yylval->str = ralloc_strdup (yyextra, yytext);
> + yylineno++;
> + yycolumn = 0;
> + if(parser->directive_ext_list) {
> +   return EXTENSION_DIRECTIVE;
> +} else {
> +   return O

Re: [Mesa-dev] [PATCH 1/2] Allow for GLSL shaders to have #extension directive anywhere in source file.

[Rogovin, Kevin]

Hi,

 I appreciate the feedback, however I am having difficulty in figuring out a 
-good- way to break up the first patch. The basic beans is that all of it's 
parts are needed together for it to even make sense. To be precise, the changes 
consist of:
  1) implementation of linked list of #extension directives (glcpp_extras.[ch] )
  2) addition to glsl_parser_extras.cpp (and .h) using it
  3) changes to glcpp-parse.y and glcpp-lex.l, checking for the new field and 
using it.

Until it is active (i.e. the changes in  glsl_parser_extras.cpp passing it to 
the glcpp), all the code
added is more or less dead code; in particular if the patch is broken up, the 
patch that turned it
on would appear to be the guilty party but when in fact it was a previous patch.

What is a good way to break up the patch so that the code added is not dead and 
so that
finding a bug in the added code can be done on a patch by patch basis?

Best Regards
 -Kevin Rogovin




From: Kenneth Graunke [kenn...@whitecape.org]
Sent: Tuesday, December 03, 2013 10:49 AM
To: Rogovin, Kevin; mesa-dev@lists.freedesktop.org
Subject: Re: [Mesa-dev] [PATCH 1/2] Allow for GLSL shaders to have #extension 
directive anywhere in source file.

On 12/03/2013 12:00 AM, Kevin Rogovin wrote:
> Allow for GLSL shaders to have #extension directive
> anywhere in source file; during preprocessor, #extension
> directives are saved to a list rather than being added
> to the pre-processed string. That list is played back
> before parsing of pre-processed text takes place.
>
> ---
>  src/glsl/Makefile.sources   |   3 +-
>  src/glsl/glcpp/glcpp-lex.l  |  19 +-
>  src/glsl/glcpp/glcpp-parse.y|  31 +-
>  src/glsl/glcpp/glcpp.c  |   2 +-
>  src/glsl/glcpp/glcpp.h  |  17 +-
>  src/glsl/glcpp/glcpp_extras.c   | 126 
> 
>  src/glsl/glcpp/glcpp_extras.h   |  97 +++
>  src/glsl/glcpp/pp.c |   5 +-
>  src/glsl/glsl_parser_extras.cpp |  55 +-
>  src/glsl/glsl_parser_extras.h   |  14 -
>  10 files changed, 358 insertions(+), 11 deletions(-)
>  create mode 100644 src/glsl/glcpp/glcpp_extras.c
>  create mode 100644 src/glsl/glcpp/glcpp_extras.h

Hi Kevin,

This is a single patch that performs a ton of refactoring as well as a
functional change.  It would be great if you could break it down into
smaller parts.  For example, perhaps you could begin parsing and
tracking extension directives, but not change how they're actually
processed.  Then, you could change it to handle them in glcpp, but still
require them to come first.  Finally, you could change the behavior to
accept them anywhere.

Submitting smaller patches makes it easier to review them.  It also
makes bisecting more accurate, for tracking down any bugs.  Finally, it
means that if you have a contentious part that might require some
rework, you can get review and possibly land the rest of your series,
and not have to respin it.

A couple of other comments:

Please follow the style of the existing glcpp code.  Annoyingly, glcpp
currently uses Carl's style, with is different than the rest of Mesa.
However, it's internally very consistent and clean, and I'd prefer to
keep it that way.

If you use vim, you can accomplish the appropriate indent settings by
putting this in your .vimrc file:

if has("autocmd")
au BufNewFile,BufRead */mesa/src/glsl/glcpp/* set noexpandtab
tabstop=8 softtabstop=8 shiftwidth=8
au BufNewFile,BufRead */mesa/* set expandtab tabstop=8
softtabstop=3 shiftwidth=3
endif

(I'm sure there's appropriate magic for Emacs or other editors as well.)

Other than indentation, a couple of nits:

/* Normal multiline comments should have
 * stars at the beginning of lines, even in the middle
 */

/**
 * Doxygen should look like this.
 *
 * with the double star at the top and stars in the middle.
 */

Please put spaces around operators and between if/while/for and parens.
 For example:

   while(current!=NULL) {  /* wrong, vs. */
   while (current != NULL) {   /* correct */

   cnt=parser->directive_ext_list->count;   /* wrong */
   cnt = parser->directive_ext_list->count; /* correct */

   for(current=state->ext_directive_list->head; current!=NULL;
current=current->next) {   /* wrong */

Also, this is the first use of MALLOC, FREE, and so on in the compiler
code.  These are stupid #defines for malloc, free, and so on.  Please
just use the standard lowercase C library functions.

Until this patch series is split up and in a more consistent style, I
don't plan to review it further.

--Ken

> diff --git a/src/glsl/Makefile.sources b/src/glsl/Makefile.sources
> index 2e81ded..90f57ad 100644
> --- a/src/glsl/Makefile.sources
> +++ b/src/glsl/Makefile.sources
> @@ -7,7 +7,8 @@ GLSL_BUILDDIR = $(top_builddir)/src/glsl
>
>  LIBGLCPP_FILES = \
>   $(GLSL_SRCDIR)/ralloc.c \
> - $(GLSL_SRCDIR)/glcpp/pp.c
> + $(GLSL_SRCDIR)/g

Re: [Mesa-dev] [PATCH 06/23] i965: Define common register base class shared between both back-ends.

[Francisco Jerez]

Petri Latvala  writes:

> On 12/02/2013 10:36 PM, Francisco Jerez wrote:
>> Would you prefer 'this->operator=(reg);'?
>>
>> I just remembered...  The reason this wouldn't work is that it would
>> trigger an implicit conversion from 'backend_reg' to 'fs_reg', causing
>> infinite recursion into the fs_reg constructor.
>>
>>
>
> For the record, this->backend_reg::operator=(reg) would have done what 
> you aimed for.
>
Right, that would have worked too.

> Not saying it kosher either way though, calling assignment operator in a 
> constructor is iffy :P. The proper way is selecting the correct base 
> class constructor to call.
>
>
> -- 
> Petri Latvala
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev


pgpyZjeiMRqOH.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 1/2] Allow for GLSL shaders to have #extension directive anywhere in source file.

[Kenneth Graunke]

On 12/03/2013 01:00 AM, Rogovin, Kevin wrote:
> Hi,
> 
>  I appreciate the feedback, however I am having difficulty in figuring out a 
> -good- way to break up the first patch. The basic beans is that all of it's 
> parts are needed together for it to even make sense. To be precise, the 
> changes consist of:
>   1) implementation of linked list of #extension directives 
> (glcpp_extras.[ch] )
>   2) addition to glsl_parser_extras.cpp (and .h) using it
>   3) changes to glcpp-parse.y and glcpp-lex.l, checking for the new field and 
> using it.
> 
> Until it is active (i.e. the changes in  glsl_parser_extras.cpp passing it to 
> the glcpp), all the code
> added is more or less dead code; in particular if the patch is broken up, the 
> patch that turned it
> on would appear to be the guilty party but when in fact it was a previous 
> patch.
> 
> What is a good way to break up the patch so that the code added is not dead 
> and so that
> finding a bug in the added code can be done on a patch by patch basis?
> 
> Best Regards
>  -Kevin Rogovin

Yeah...there's a point where you switch from the old code to the new
code, and most bugs would probably bisect to that.  In this case, I
think that's fairly unavoidable.

However, splitting it up into multiple patches would definitely make it
a lot easier to review.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 1/2] Allow for GLSL shaders to have #extension directive anywhere in source file.

[Kenneth Graunke]

On 12/03/2013 12:00 AM, Kevin Rogovin wrote:
[snip]
> diff --git a/src/glsl/glcpp/glcpp_extras.c b/src/glsl/glcpp/glcpp_extras.c
> new file mode 100644
> index 000..1672f9f
> --- /dev/null
> +++ b/src/glsl/glcpp/glcpp_extras.c
> @@ -0,0 +1,126 @@
> +#include 
> +#include 
> +#include "glcpp_extras.h"
> +#include "main/imports.h"
> +
> +#define MAX_STRING_LENGTH 200
> +
> +static const char*
> +remove_surrounding_white_space(char *value)
> +{
> +   size_t len;
> +   char *back;
> +
> +   /*
> + remark:
> +  -since the return value of remove_surrounding_white_space
> +   is to never be freed, we can safely return a non-NULL
> +   "empty" string if value was NULL.
> +*/
> +
> +   if(value==NULL || *value==0) {
> +  return "";
> +   }
> +
> +   while(*value!=0 && isspace(*value)) {
> +  ++value;
> +   }
> +
> +   len=strlen(value);
> +   if(len==0) {
> +  return value;
> +   }
> +
> +   for(back = value + len - 1; back!=value && isspace(*back); --back) {
> +  *back=0;
> +   }
> +
> +   return value;
> +}
> +
> +static char*
> +copy_string(const char *src)
> +{
> +   char *dst;
> +   size_t len;
> +
> +   len=strnlen(src, MAX_STRING_LENGTH);
> +   dst=MALLOC(len+1);
> +   strncpy(dst, src, len);
> +
> +   dst[len]=0;
> +   return dst;
> +}

Isn't this just strdup()?  (Other than it being broken for strings
longer than an arbitrary limit of 200 characters...)

> +
> +static
> +void
> +delete_glcpp_extension_directive_entry(struct 
> glcpp_extension_directive_list_element *p)
> +{
> +   FREE(p->line_string);
> +   FREE(p->strtoked_string);
> +   FREE(p);
> +}

I bet you could allocate p->line_string and p->strtoked_string using
ralloc---maybe ralloc_strdup(p, string_to_copy)?  Then ralloc_free(p)
would take care of the contents, and you wouldn't need to write this
function.

> +void 
> +init_glcpp_extension_directive_list(struct glcpp_extension_directive_list *p)
> +{
> +   assert(p!=NULL);
> +   p->head=p->tail=NULL;
> +   p->count=0;
> +}
> +
> +void 
> +delete_glcpp_extension_directive_list_contents(struct 
> glcpp_extension_directive_list *p)
> +{
> +   struct glcpp_extension_directive_list_element *current;
> +
> +   assert(p!=NULL);
> +   current=p->head;
> +   while(current!=NULL) {
> +  struct glcpp_extension_directive_list_element *next;
> +
> +  next=current->next;
> +  delete_glcpp_extension_directive_entry(current);
> +  current=next;
> +   }
> +   p->head=p->tail=NULL;
> +   p->count=0;
> +}

...and if you used ralloc to allocate nodes, then you could probably
just free the top-level list and not have to write this either.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 1/2] Allow for GLSL shaders to have #extension directive anywhere in source file.

[Rogovin, Kevin]

Hi,


> Yeah...there's a point where you switch from the old code to the new
> code, and most bugs would probably bisect to that.  In this case, I
> think that's fairly unavoidable.
>
> However, splitting it up into multiple patches would definitely make it
> a lot easier to review.

So, is it required to do the split or not? My opinion is that it is easier to
review as one patch as it keeps the context of the changes too. I think
it is a very BAD^tm thing when a patch adds dead code, it makes me 
shudder. I also freely admit that I do think that the patch I submitted was
small-ish in terms of scale: the refactoring was tiny, and so on. Indeed
I have seen patches that change much more get the OK as well. 

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

Re: [Mesa-dev] [PATCH 1/2] Allow for GLSL shaders to have #extension directive anywhere in source file.

[Rogovin, Kevin]

Hi,

 Thankyou for the beans on ralloc. I will send out a patch sequence shortly 
that:
   1) fixes the formatting issues (spaces, comments, etc)
   2) uses ralloc in glcpp_extras.c

Best Regards
- Kevin Rogovin


From: Kenneth Graunke [kenn...@whitecape.org]
Sent: Tuesday, December 03, 2013 11:44 AM
To: Rogovin, Kevin; mesa-dev@lists.freedesktop.org
Subject: Re: [Mesa-dev] [PATCH 1/2] Allow for GLSL shaders to have #extension 
directive anywhere in source file.

On 12/03/2013 12:00 AM, Kevin Rogovin wrote:
[snip]
> diff --git a/src/glsl/glcpp/glcpp_extras.c b/src/glsl/glcpp/glcpp_extras.c
> new file mode 100644
> index 000..1672f9f
> --- /dev/null
> +++ b/src/glsl/glcpp/glcpp_extras.c
> @@ -0,0 +1,126 @@
> +#include 
> +#include 
> +#include "glcpp_extras.h"
> +#include "main/imports.h"
> +
> +#define MAX_STRING_LENGTH 200
> +
> +static const char*
> +remove_surrounding_white_space(char *value)
> +{
> +   size_t len;
> +   char *back;
> +
> +   /*
> + remark:
> +  -since the return value of remove_surrounding_white_space
> +   is to never be freed, we can safely return a non-NULL
> +   "empty" string if value was NULL.
> +*/
> +
> +   if(value==NULL || *value==0) {
> +  return "";
> +   }
> +
> +   while(*value!=0 && isspace(*value)) {
> +  ++value;
> +   }
> +
> +   len=strlen(value);
> +   if(len==0) {
> +  return value;
> +   }
> +
> +   for(back = value + len - 1; back!=value && isspace(*back); --back) {
> +  *back=0;
> +   }
> +
> +   return value;
> +}
> +
> +static char*
> +copy_string(const char *src)
> +{
> +   char *dst;
> +   size_t len;
> +
> +   len=strnlen(src, MAX_STRING_LENGTH);
> +   dst=MALLOC(len+1);
> +   strncpy(dst, src, len);
> +
> +   dst[len]=0;
> +   return dst;
> +}

Isn't this just strdup()?  (Other than it being broken for strings
longer than an arbitrary limit of 200 characters...)

> +
> +static
> +void
> +delete_glcpp_extension_directive_entry(struct 
> glcpp_extension_directive_list_element *p)
> +{
> +   FREE(p->line_string);
> +   FREE(p->strtoked_string);
> +   FREE(p);
> +}

I bet you could allocate p->line_string and p->strtoked_string using
ralloc---maybe ralloc_strdup(p, string_to_copy)?  Then ralloc_free(p)
would take care of the contents, and you wouldn't need to write this
function.

> +void
> +init_glcpp_extension_directive_list(struct glcpp_extension_directive_list *p)
> +{
> +   assert(p!=NULL);
> +   p->head=p->tail=NULL;
> +   p->count=0;
> +}
> +
> +void
> +delete_glcpp_extension_directive_list_contents(struct 
> glcpp_extension_directive_list *p)
> +{
> +   struct glcpp_extension_directive_list_element *current;
> +
> +   assert(p!=NULL);
> +   current=p->head;
> +   while(current!=NULL) {
> +  struct glcpp_extension_directive_list_element *next;
> +
> +  next=current->next;
> +  delete_glcpp_extension_directive_entry(current);
> +  current=next;
> +   }
> +   p->head=p->tail=NULL;
> +   p->count=0;
> +}

...and if you used ralloc to allocate nodes, then you could probably
just free the top-level list and not have to write this either.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH 1/2] Allow for GLSL shaders to have #extension directive anywhere in source file. (reworked)

[Kevin Rogovin]

Allow for GLSL shaders to have #extension directive
anywhere in source file; during preprocessor, #extension
directives are saved to a list rather than being added
to the pre-processed string. That list is played back 
before parsing of pre-processed text takes place.  

---
 src/glsl/Makefile.sources   |   3 +-
 src/glsl/glcpp/glcpp-lex.l  |  20 +++-
 src/glsl/glcpp/glcpp-parse.y|  30 ++-
 src/glsl/glcpp/glcpp.c  |   2 +-
 src/glsl/glcpp/glcpp.h  |  17 +-
 src/glsl/glcpp/glcpp_extras.c   | 111 
 src/glsl/glcpp/glcpp_extras.h   |  97 +++
 src/glsl/glcpp/pp.c |   5 +-
 src/glsl/glsl_parser_extras.cpp |  56 +++-
 src/glsl/glsl_parser_extras.h   |  14 -
 10 files changed, 344 insertions(+), 11 deletions(-)
 create mode 100644 src/glsl/glcpp/glcpp_extras.c
 create mode 100644 src/glsl/glcpp/glcpp_extras.h

diff --git a/src/glsl/Makefile.sources b/src/glsl/Makefile.sources
index 2e81ded..90f57ad 100644
--- a/src/glsl/Makefile.sources
+++ b/src/glsl/Makefile.sources
@@ -7,7 +7,8 @@ GLSL_BUILDDIR = $(top_builddir)/src/glsl
 
 LIBGLCPP_FILES = \
$(GLSL_SRCDIR)/ralloc.c \
-   $(GLSL_SRCDIR)/glcpp/pp.c
+   $(GLSL_SRCDIR)/glcpp/pp.c \
+$(GLSL_SRCDIR)/glcpp/glcpp_extras.c
 
 LIBGLCPP_GENERATED_FILES = \
$(GLSL_BUILDDIR)/glcpp/glcpp-lex.c \
diff --git a/src/glsl/glcpp/glcpp-lex.l b/src/glsl/glcpp/glcpp-lex.l
index a029f62..1998fce 100644
--- a/src/glsl/glcpp/glcpp-lex.l
+++ b/src/glsl/glcpp/glcpp-lex.l
@@ -126,15 +126,31 @@ HEXADECIMAL_INTEGER   0[xX][0-9a-fA-F]+[uU]?
return HASH_VERSION;
 }
 
-   /* glcpp doesn't handle #extension, #version, or #pragma directives.
+   /* glcpp doesn't handle #pragma directives.
 * Simply pass them through to the main compiler's lexer/parser. */
-{HASH}(extension|pragma)[^\n]+ {
+{HASH}(pragma)[^\n]+ {
yylval->str = ralloc_strdup (yyextra, yytext);
yylineno++;
yycolumn = 0;
return OTHER;
 }
 
+/* glcpp will handle the #extension directive if
+* and only if parser->directive_ext_list is non-NULL
+* otherwise it will just send that off to the 
+* main compiler's lexer/parser. */
+{HASH}(extension)[^\n]+ {
+yylval->str = ralloc_strdup (yyextra, yytext);
+   yylineno++;
+   yycolumn = 0;
+   if(parser->directive_ext_list) {
+   return EXTENSION_DIRECTIVE;
+} else {
+   return OTHER;
+   }
+}
+
+
 {HASH}line{HSPACE}+ {
return HASH_LINE;
 }
diff --git a/src/glsl/glcpp/glcpp-parse.y b/src/glsl/glcpp/glcpp-parse.y
index 7edc274..b5a5f05 100644
--- a/src/glsl/glcpp/glcpp-parse.y
+++ b/src/glsl/glcpp/glcpp-parse.y
@@ -166,6 +166,7 @@ add_builtin_define(glcpp_parser_t *parser, const char 
*name, int value);
 %expect 0
 %token COMMA_FINAL DEFINED ELIF_EXPANDED HASH HASH_DEFINE FUNC_IDENTIFIER 
OBJ_IDENTIFIER HASH_ELIF HASH_ELSE HASH_ENDIF HASH_IF HASH_IFDEF HASH_IFNDEF 
HASH_LINE HASH_UNDEF HASH_VERSION IDENTIFIER IF_EXPANDED INTEGER INTEGER_STRING 
LINE_EXPANDED NEWLINE OTHER PLACEHOLDER SPACE
 %token PASTE
+%token  EXTENSION_DIRECTIVE
 %type  expression INTEGER operator SPACE integer_constant
 %type  IDENTIFIER FUNC_IDENTIFIER OBJ_IDENTIFIER INTEGER_STRING OTHER
 %type  identifier_list
@@ -208,9 +209,33 @@ line:
ralloc_free ($1);
}
 |  expanded_line
+|   extension_directive
 |  HASH non_directive
 ;
 
+extension_directive:
+EXTENSION_DIRECTIVE {
+int cnt;
+   
+assert(parser->directive_ext_list);
+/* the lexer increments yylineno when it encounters
+ * #extension. If it did not, then the line used by
+ * the glsl parser when then be wrong. We get around
+ * the issue by decrementing by number of #extension
+ * processed so far.
+*/
+cnt=parser->directive_ext_list->count;
+add_extension_from_preprocessor($1, 
+@1.first_line - cnt, 
+@1.first_column,
+@1.last_line - cnt,
+@1.last_column,
+@1.source,
+parser->directive_ext_list); 
+}
+;
+
+
 expanded_line:
IF_EXPANDED expression NEWLINE {
_glcpp_parser_skip_stack_push_if (parser, & @1, $2);
@@ -1148,7 +1173,8 @@ static void add_builtin_define(glcpp_parser_t *parser,
 }
 
 glcpp_parser_t *
-glcpp_parser_create (const struct gl_extensions *extensions, int api)
+glcpp_parser_create (const struct gl_extensions *extensions, int api,
+ struct glcpp_extension_directive_list *ext_list)

[Mesa-dev] [PATCH 2/2] add MESA_GLSL option to require that #extension directives come before any variable or function declarations, etc (reworked)

[Kevin Rogovin]

Add an additional bit flag, GLSL_EXTENSION_STRICT for 
gl_context#Shader#Flags to require that #extension 
directives must come before variable or function
declarations. The flag is set to on if MESA_GLSL has
the option extension_strict_ordering in it.

---
 src/glsl/glsl_parser_extras.cpp | 7 ---
 src/mesa/main/mtypes.h  | 8 +++-
 src/mesa/main/shaderapi.c   | 2 ++
 3 files changed, 13 insertions(+), 4 deletions(-)

diff --git a/src/glsl/glsl_parser_extras.cpp b/src/glsl/glsl_parser_extras.cpp
index 7ac6798..b74cc83 100644
--- a/src/glsl/glsl_parser_extras.cpp
+++ b/src/glsl/glsl_parser_extras.cpp
@@ -1517,11 +1517,12 @@ _mesa_glsl_compile_shader(struct gl_context *ctx, 
struct gl_shader *shader,
   new(shader) _mesa_glsl_parse_state(ctx, shader->Type, shader);
const char *source = shader->Source;
 
-   if (1) {
+
+   if (ctx->Shader.Flags&GLSL_EXTENSION_STRICT) {
+  state->ext_directive_list = NULL;
+   } else {
   state->ext_directive_list = &extension_directive_list;
   init_glcpp_extension_directive_list(state->ext_directive_list);
-   } else {
-  state->ext_directive_list = NULL;
}
 
state->error = glcpp_preprocess(state, &source, &state->info_log,
diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h
index b4b432f..28cb5f9 100644
--- a/src/mesa/main/mtypes.h
+++ b/src/mesa/main/mtypes.h
@@ -2652,7 +2652,13 @@ struct gl_shader_program
 #define GLSL_USE_PROG 0x80  /**< Log glUseProgram calls */
 #define GLSL_REPORT_ERRORS 0x100  /**< Print compilation errors */
 #define GLSL_DUMP_ON_ERROR 0x200 /**< Dump shaders to stderr on compile error 
*/
-
+/**
+ * In GLSL sources, follow specifications 
+ * with respect to #extension ordering, i.e.
+ * such directives must come before function
+ * declarations, variable declarations, etc.
+ */
+#define GLSL_EXTENSION_STRICT 0x400 
 
 /**
  * Context state for GLSL vertex/fragment shaders.
diff --git a/src/mesa/main/shaderapi.c b/src/mesa/main/shaderapi.c
index 1d9aac3..c9a86e9 100644
--- a/src/mesa/main/shaderapi.c
+++ b/src/mesa/main/shaderapi.c
@@ -92,6 +92,8 @@ get_shader_flags(void)
  flags |= GLSL_USE_PROG;
   if (strstr(env, "errors"))
  flags |= GLSL_REPORT_ERRORS;
+  if (strstr(env, "extension_strict_ordering"))
+ flags |= GLSL_EXTENSION_STRICT;
}
 
return flags;
-- 
1.8.1.2

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

Re: [Mesa-dev] [PATCH 1/2] glsl: extract function for record comparisons

[Grigori Goronzy]

Ping? Can anyone review this, please?

Grigori

On 27.11.2013 00:15, Grigori Goronzy wrote:

---
  src/glsl/glsl_types.cpp | 61 +++--
  src/glsl/glsl_types.h   |  7 ++
  2 files changed, 41 insertions(+), 27 deletions(-)

diff --git a/src/glsl/glsl_types.cpp b/src/glsl/glsl_types.cpp
index f740130..6c9727e 100644
--- a/src/glsl/glsl_types.cpp
+++ b/src/glsl/glsl_types.cpp
@@ -447,6 +447,39 @@ glsl_type::get_array_instance(const glsl_type *base, 
unsigned array_size)
  }


+bool
+glsl_type::record_compare(const glsl_type *b) const
+{
+   if (this->length != b->length)
+  return false;
+
+   if (this->interface_packing != b->interface_packing)
+  return false;
+
+   for (unsigned i = 0; i < this->length; i++) {
+  if (this->fields.structure[i].type != b->fields.structure[i].type)
+return false;
+  if (strcmp(this->fields.structure[i].name,
+b->fields.structure[i].name) != 0)
+return false;
+  if (this->fields.structure[i].row_major
+ != b->fields.structure[i].row_major)
+return false;
+  if (this->fields.structure[i].location
+  != b->fields.structure[i].location)
+ return false;
+  if (this->fields.structure[i].interpolation
+  != b->fields.structure[i].interpolation)
+ return false;
+  if (this->fields.structure[i].centroid
+  != b->fields.structure[i].centroid)
+ return false;
+   }
+
+   return true;
+}
+
+
  int
  glsl_type::record_key_compare(const void *a, const void *b)
  {
@@ -459,33 +492,7 @@ glsl_type::record_key_compare(const void *a, const void *b)
 if (strcmp(key1->name, key2->name) != 0)
return 1;

-   if (key1->length != key2->length)
-  return 1;
-
-   if (key1->interface_packing != key2->interface_packing)
-  return 1;
-
-   for (unsigned i = 0; i < key1->length; i++) {
-  if (key1->fields.structure[i].type != key2->fields.structure[i].type)
-return 1;
-  if (strcmp(key1->fields.structure[i].name,
-key2->fields.structure[i].name) != 0)
-return 1;
-  if (key1->fields.structure[i].row_major
- != key2->fields.structure[i].row_major)
-return 1;
-  if (key1->fields.structure[i].location
-  != key2->fields.structure[i].location)
- return 1;
-  if (key1->fields.structure[i].interpolation
-  != key2->fields.structure[i].interpolation)
- return 1;
-  if (key1->fields.structure[i].centroid
-  != key2->fields.structure[i].centroid)
- return 1;
-   }
-
-   return 0;
+   return !key1->record_compare(key2);
  }


diff --git a/src/glsl/glsl_types.h b/src/glsl/glsl_types.h
index 96eee5e..dcc98e0 100644
--- a/src/glsl/glsl_types.h
+++ b/src/glsl/glsl_types.h
@@ -542,6 +542,13 @@ struct glsl_type {
  */
 int sampler_coordinate_components() const;

+   /**
+* Compare a record type against another record type.
+*
+* This is useful for matching record types declared across shader stages.
+*/
+   bool record_compare(const glsl_type *b) const;
+
  private:
 /**
  * ralloc context for all glsl_type allocations



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

Re: [Mesa-dev] [PATCH 1/4] glsl: Extract functions from loop_analysis::visit(ir_dereference_variable *).

[Pohjolainen, Topi]

On Thu, Nov 28, 2013 at 11:41:06AM -0800, Paul Berry wrote:
> This function is about to get more complex.
> ---
>  src/glsl/loop_analysis.cpp | 91 
> +-
>  src/glsl/loop_analysis.h   |  4 ++
>  2 files changed, 70 insertions(+), 25 deletions(-)
> 
> diff --git a/src/glsl/loop_analysis.cpp b/src/glsl/loop_analysis.cpp
> index b08241a..3f2a14b 100644
> --- a/src/glsl/loop_analysis.cpp
> +++ b/src/glsl/loop_analysis.cpp
> @@ -33,6 +33,43 @@ static bool 
> all_expression_operands_are_loop_constant(ir_rvalue *,
>  static ir_rvalue *get_basic_induction_increment(ir_assignment *, hash_table 
> *);
>  
>  
> +/**
> + * Record the fact that the given loop variable was referenced inside the 
> loop.
> + *
> + * \arg in_assignee is true if the reference was on the LHS of an assignment.
> + *
> + * \arg in_conditional_code is true if the reference occurred inside an if
> + * statement.
> + *
> + * \arg current_assignment is the ir_assignment node that the loop variable 
> is
> + * on the LHS of, if any (ignored if \c in_assignee is false).
> + */
> +void
> +loop_variable::record_reference(bool in_assignee, bool in_conditional_code,
> +ir_assignment *current_assignment)
> +{
> +   if (in_assignee) {
> +  assert(current_assignment != NULL);
> +
> +  this->conditional_assignment = in_conditional_code
> + || current_assignment->condition != NULL;
> +
> +  if (this->first_assignment == NULL) {
> + assert(this->num_assignments == 0);
> +
> + this->first_assignment = current_assignment;
> +  }
> +
> +  this->num_assignments++;
> +   } else if (this->first_assignment == current_assignment) {
> +  /* This catches the case where the variable is used in the RHS of an
> +   * assignment where it is also in the LHS.
> +   */
> +  this->read_before_write = true;
> +   }
> +}
> +
> +
>  loop_state::loop_state()
>  {
> this->ht = hash_table_ctor(0, hash_table_pointer_hash,
> @@ -102,6 +139,32 @@ loop_variable_state::insert(ir_if *if_stmt)
> return t;
>  }
>  
> +
> +/**
> + * If the given variable already is recored in the state for this loop, 
> return
^
recorded

> + * the corresponding loop_variable object that records information about it.
> + *
> + * Otherwise, create a new loop_variable object to record information about
> + * the variable, and set its \c read_before_write field appropriately based 
> on
> + * \c in_assignee.
> + *
> + * \arg in_assignee is true if this variable was encountered on the LHS of an
> + * assignment.
> + */
> +loop_variable *
> +loop_variable_state::get_or_insert(ir_variable *var, bool in_assignee)
> +{
> +   loop_variable *lv = this->get(var);
> +
> +   if (lv == NULL) {
> +  lv = this->insert(var);
> +  lv->read_before_write = !in_assignee;
> +   }
> +
> +   return lv;
> +}
> +
> +
>  namespace {
>  
>  class loop_analysis : public ir_hierarchical_visitor {
> @@ -181,32 +244,10 @@ loop_analysis::visit(ir_dereference_variable *ir)
>(loop_variable_state *) this->state.get_head();
>  
> ir_variable *var = ir->variable_referenced();
> -   loop_variable *lv = ls->get(var);
> -
> -   if (lv == NULL) {
> -  lv = ls->insert(var);
> -  lv->read_before_write = !this->in_assignee;
> -   }
> -
> -   if (this->in_assignee) {
> -  assert(this->current_assignment != NULL);
> +   loop_variable *lv = ls->get_or_insert(var, this->in_assignee);
>  
> -  lv->conditional_assignment = (this->if_statement_depth > 0)
> -  || (this->current_assignment->condition != NULL);
> -
> -  if (lv->first_assignment == NULL) {
> -  assert(lv->num_assignments == 0);
> -
> -  lv->first_assignment = this->current_assignment;
> -  }
> -
> -  lv->num_assignments++;
> -   } else if (lv->first_assignment == this->current_assignment) {
> -  /* This catches the case where the variable is used in the RHS of an
> -   * assignment where it is also in the LHS.
> -   */
> -  lv->read_before_write = true;
> -   }
> +   lv->record_reference(this->in_assignee, this->if_statement_depth > 0,
> +this->current_assignment);
>  
> return visit_continue;
>  }
> diff --git a/src/glsl/loop_analysis.h b/src/glsl/loop_analysis.h
> index 769d626..edad8f3 100644
> --- a/src/glsl/loop_analysis.h
> +++ b/src/glsl/loop_analysis.h
> @@ -67,6 +67,7 @@ class loop_variable_state : public exec_node {
>  public:
> class loop_variable *get(const ir_variable *);
> class loop_variable *insert(ir_variable *);
> +   class loop_variable *get_or_insert(ir_variable *, bool in_assignee);
> class loop_terminator *insert(ir_if *);
>  
>  
> @@ -217,6 +218,9 @@ public:
>  
>return is_const;
> }
> +
> +   void record_reference(bool in_assignee, bool in_conditional_code,
> + ir_assignment *current_assignment);
>  };
>  
>  
> -- 
> 1.8.4.2
> 
> __

Re: [Mesa-dev] [PATCH 1/2] Allow for GLSL shaders to have #extension directive anywhere in source file. (reworked)

[Brian Paul]

On 12/03/2013 05:47 AM, Kevin Rogovin wrote:

Allow for GLSL shaders to have #extension directive
anywhere in source file; during preprocessor, #extension
directives are saved to a list rather than being added
to the pre-processed string. That list is played back
before parsing of pre-processed text takes place.



Kevin,  I have a patch that accomplishes the same thing via a DRI config 
option.  I'm still catching up on my email backlog but I'll

try to post it later.

-Brian

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

Re: [Mesa-dev] [PATCH 1/3] glx: Check malloc return value before accessing memory in glx/clientattrib.c

[Brian Paul]

On 12/02/2013 02:39 AM, Juha-Pekka Heikkila wrote:

Signed-off-by: Juha-Pekka Heikkila 
---
  src/glx/clientattrib.c | 5 +
  1 file changed, 5 insertions(+)

diff --git a/src/glx/clientattrib.c b/src/glx/clientattrib.c
index 1b306ea..a26906f 100644
--- a/src/glx/clientattrib.c
+++ b/src/glx/clientattrib.c
@@ -76,6 +76,11 @@ __indirect_glPushClientAttrib(GLuint mask)
 if (spp < &gc->attributes.stack[__GL_CLIENT_ATTRIB_STACK_DEPTH]) {
if (!(sp = *spp)) {
   sp = malloc(sizeof(__GLXattribute));
+
+ if (sp == NULL) {
+__glXSetError(gc, GL_OUT_OF_MEMORY);
+return;
+ }
   *spp = sp;
}
sp->mask = mask;



Reviewed-by: Brian Paul 

Need someone to push this for you?

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

Re: [Mesa-dev] [PATCH 2/3] mesa: Verify memory allocations success in _mesa_PushClientAttrib

[Brian Paul]

On 12/02/2013 02:39 AM, Juha-Pekka Heikkila wrote:

Check if any of the callocs fail and report it with _mesa_error
if needed.

Signed-off-by: Juha-Pekka Heikkila 
---
  src/mesa/main/attrib.c | 34 ++
  1 file changed, 30 insertions(+), 4 deletions(-)

diff --git a/src/mesa/main/attrib.c b/src/mesa/main/attrib.c
index c9332bd..2418fb0 100644
--- a/src/mesa/main/attrib.c
+++ b/src/mesa/main/attrib.c
@@ -1488,6 +1488,12 @@ init_array_attrib_data(struct gl_context *ctx,
  {
 /* Get a non driver gl_array_object. */
 attrib->ArrayObj = CALLOC_STRUCT( gl_array_object );
+
+   if (attrib->ArrayObj == NULL) {
+  _mesa_error(ctx, GL_OUT_OF_MEMORY, "glPushClientAttrib");
+  return;
+   }
+


This is good, but if CALLOC_STRUCT() fails we'd crash anyway because the 
following call to save_array_attrib() would dereference the null 
pointer.  init_array_attrib_data() should probably return a true/false 
success/failure result so the following array attrib calls could be skipped.




 _mesa_initialize_array_object(ctx, attrib->ArrayObj, 0);
  }

@@ -1516,7 +1522,7 @@ _mesa_PushClientAttrib(GLbitfield mask)
 GET_CURRENT_CONTEXT(ctx);

 if (ctx->ClientAttribStackDepth >= MAX_CLIENT_ATTRIB_STACK_DEPTH) {
-  _mesa_error( ctx, GL_STACK_OVERFLOW, "glPushClientAttrib" );
+  _mesa_error(ctx, GL_STACK_OVERFLOW, "glPushClientAttrib");
return;
 }

@@ -1529,10 +1535,19 @@ _mesa_PushClientAttrib(GLbitfield mask)
struct gl_pixelstore_attrib *attr;
/* packing attribs */
attr = CALLOC_STRUCT( gl_pixelstore_attrib );
+  if (attr == NULL) {
+ _mesa_error(ctx, GL_OUT_OF_MEMORY, "glPushClientAttrib");
+ goto end;
+  }
copy_pixelstore(ctx, attr, &ctx->Pack);
save_attrib_data(&head, GL_CLIENT_PACK_BIT, attr);
/* unpacking attribs */
attr = CALLOC_STRUCT( gl_pixelstore_attrib );
+  if (attr == NULL) {
+ _mesa_error(ctx, GL_OUT_OF_MEMORY, "glPushClientAttrib");
+ goto end;
+  }
+
copy_pixelstore(ctx, attr, &ctx->Unpack);
save_attrib_data(&head, GL_CLIENT_UNPACK_BIT, attr);
 }
@@ -1540,13 +1555,24 @@ _mesa_PushClientAttrib(GLbitfield mask)
 if (mask & GL_CLIENT_VERTEX_ARRAY_BIT) {
struct gl_array_attrib *attr;
attr = CALLOC_STRUCT( gl_array_attrib );
+  if (attr == NULL) {
+ _mesa_error(ctx, GL_OUT_OF_MEMORY, "glPushClientAttrib");
+ goto end;
+  }
+
init_array_attrib_data(ctx, attr);
+  if (attr->ArrayObj == NULL) {
+  goto end;
+  }
+
save_array_attrib(ctx, attr, &ctx->Array);
save_attrib_data(&head, GL_CLIENT_VERTEX_ARRAY_BIT, attr);
 }
-
-   ctx->ClientAttribStack[ctx->ClientAttribStackDepth] = head;
-   ctx->ClientAttribStackDepth++;
+end:
+   if (head != NULL) {
+   ctx->ClientAttribStack[ctx->ClientAttribStackDepth] = head;
+   ctx->ClientAttribStackDepth++;
+   }
  }


The rest looks OK.

-Brian


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

Re: [Mesa-dev] [PATCH 1/2] Allow for GLSL shaders to have #extension directive anywhere in source file.

[Ian Romanick]

Can we please hold off on this?  I'm in the process of getting the GLSL
spec updated to match what people actually ship.  We've always taken the
approach of following the spec, and if the spec doesn't match reality,
fix the spec.  I've done this about a dozen times over the last 3 years,
and I don't feel like there's a compelling reason to deviate for this case.

I also don't think the behavior implemented by this patch exactly
matches other vendors.  I don't want to put a set of hacks in now only
to replace them with something different in a few weeks.

On 12/03/2013 12:00 AM, Kevin Rogovin wrote:
> Allow for GLSL shaders to have #extension directive
> anywhere in source file; during preprocessor, #extension
> directives are saved to a list rather than being added
> to the pre-processed string. That list is played back 
> before parsing of pre-processed text takes place.  
> 
> ---
>  src/glsl/Makefile.sources   |   3 +-
>  src/glsl/glcpp/glcpp-lex.l  |  19 +-
>  src/glsl/glcpp/glcpp-parse.y|  31 +-
>  src/glsl/glcpp/glcpp.c  |   2 +-
>  src/glsl/glcpp/glcpp.h  |  17 +-
>  src/glsl/glcpp/glcpp_extras.c   | 126 
> 
>  src/glsl/glcpp/glcpp_extras.h   |  97 +++
>  src/glsl/glcpp/pp.c |   5 +-
>  src/glsl/glsl_parser_extras.cpp |  55 +-
>  src/glsl/glsl_parser_extras.h   |  14 -
>  10 files changed, 358 insertions(+), 11 deletions(-)
>  create mode 100644 src/glsl/glcpp/glcpp_extras.c
>  create mode 100644 src/glsl/glcpp/glcpp_extras.h
> 
> diff --git a/src/glsl/Makefile.sources b/src/glsl/Makefile.sources
> index 2e81ded..90f57ad 100644
> --- a/src/glsl/Makefile.sources
> +++ b/src/glsl/Makefile.sources
> @@ -7,7 +7,8 @@ GLSL_BUILDDIR = $(top_builddir)/src/glsl
>  
>  LIBGLCPP_FILES = \
>   $(GLSL_SRCDIR)/ralloc.c \
> - $(GLSL_SRCDIR)/glcpp/pp.c
> + $(GLSL_SRCDIR)/glcpp/pp.c \
> +$(GLSL_SRCDIR)/glcpp/glcpp_extras.c
>  
>  LIBGLCPP_GENERATED_FILES = \
>   $(GLSL_BUILDDIR)/glcpp/glcpp-lex.c \
> diff --git a/src/glsl/glcpp/glcpp-lex.l b/src/glsl/glcpp/glcpp-lex.l
> index a029f62..31b246f 100644
> --- a/src/glsl/glcpp/glcpp-lex.l
> +++ b/src/glsl/glcpp/glcpp-lex.l
> @@ -126,15 +126,30 @@ HEXADECIMAL_INTEGER 0[xX][0-9a-fA-F]+[uU]?
>   return HASH_VERSION;
>  }
>  
> - /* glcpp doesn't handle #extension, #version, or #pragma directives.
> + /* glcpp doesn't handle #pragma directives.
>* Simply pass them through to the main compiler's lexer/parser. */
> -{HASH}(extension|pragma)[^\n]+ {
> +{HASH}(pragma)[^\n]+ {
>   yylval->str = ralloc_strdup (yyextra, yytext);
>   yylineno++;
>   yycolumn = 0;
>   return OTHER;
>  }
>  
> +/* glcpp will handle the #extension directive if
> +  * and only if parser->directive_ext_list is non-NULL
> +  * otherwise it will just send that off to the parser */
> +{HASH}(extension)[^\n]+ {
> +yylval->str = ralloc_strdup (yyextra, yytext);
> + yylineno++;
> + yycolumn = 0;
> + if(parser->directive_ext_list) {
> +   return EXTENSION_DIRECTIVE;
> +} else {
> +   return OTHER;
> + }
> +}
> +
> +
>  {HASH}line{HSPACE}+ {
>   return HASH_LINE;
>  }
> diff --git a/src/glsl/glcpp/glcpp-parse.y b/src/glsl/glcpp/glcpp-parse.y
> index 7edc274..d1d58c3 100644
> --- a/src/glsl/glcpp/glcpp-parse.y
> +++ b/src/glsl/glcpp/glcpp-parse.y
> @@ -166,6 +166,7 @@ add_builtin_define(glcpp_parser_t *parser, const char 
> *name, int value);
>  %expect 0
>  %token COMMA_FINAL DEFINED ELIF_EXPANDED HASH HASH_DEFINE FUNC_IDENTIFIER 
> OBJ_IDENTIFIER HASH_ELIF HASH_ELSE HASH_ENDIF HASH_IF HASH_IFDEF HASH_IFNDEF 
> HASH_LINE HASH_UNDEF HASH_VERSION IDENTIFIER IF_EXPANDED INTEGER 
> INTEGER_STRING LINE_EXPANDED NEWLINE OTHER PLACEHOLDER SPACE
>  %token PASTE
> +%token  EXTENSION_DIRECTIVE
>  %type  expression INTEGER operator SPACE integer_constant
>  %type  IDENTIFIER FUNC_IDENTIFIER OBJ_IDENTIFIER INTEGER_STRING OTHER
>  %type  identifier_list
> @@ -208,9 +209,34 @@ line:
>   ralloc_free ($1);
>   }
>  |expanded_line
> +|   extension_directive
>  |HASH non_directive
>  ;
>  
> +extension_directive:
> +EXTENSION_DIRECTIVE {
> +   int cnt;
> +   
> +   assert(parser->directive_ext_list);
> +   /*
> + the lexer increments yylineno when it encounters
> + #extension. If it did not, then the line used by
> + the glsl parser when then be wrong. We get around
> + the issue by decrementing by number of #extension
> + processed so far.
> +*/
> +   cnt=parser->directive_ext_list->count;
> +   add_extension_from_preprocessor($1, 
> +   @1.first_line - cnt, 
> +   @1.first_column,
> +   @1.last

Re: [Mesa-dev] [PATCH 3/3] mesa: Verify memory allocations success in _mesa_PushAttrib

[Brian Paul]

On 12/02/2013 02:39 AM, Juha-Pekka Heikkila wrote:

Check if any of the callocs fail and report it with _mesa_error
if needed.

Signed-off-by: Juha-Pekka Heikkila 
---
  src/mesa/main/attrib.c | 106 -
  1 file changed, 104 insertions(+), 2 deletions(-)

diff --git a/src/mesa/main/attrib.c b/src/mesa/main/attrib.c
index 2418fb0..037cd64 100644
--- a/src/mesa/main/attrib.c
+++ b/src/mesa/main/attrib.c
@@ -222,6 +222,11 @@ _mesa_PushAttrib(GLbitfield mask)
 if (mask & GL_ACCUM_BUFFER_BIT) {
struct gl_accum_attrib *attr;
attr = MALLOC_STRUCT( gl_accum_attrib );
+  if (attr == NULL) {
+ _mesa_error(ctx, GL_OUT_OF_MEMORY, "glPushAttrib");
+ goto end;
+  }
+
memcpy( attr, &ctx->Accum, sizeof(struct gl_accum_attrib) );
save_attrib_data(&head, GL_ACCUM_BUFFER_BIT, attr);
 }
@@ -230,6 +235,11 @@ _mesa_PushAttrib(GLbitfield mask)
GLuint i;
struct gl_colorbuffer_attrib *attr;
attr = MALLOC_STRUCT( gl_colorbuffer_attrib );
+  if (attr == NULL) {
+ _mesa_error(ctx, GL_OUT_OF_MEMORY, "glPushAttrib");
+ goto end;
+  }
+
memcpy( attr, &ctx->Color, sizeof(struct gl_colorbuffer_attrib) );
/* push the Draw FBO's DrawBuffer[] state, not ctx->Color.DrawBuffer[] 
*/
for (i = 0; i < ctx->Const.MaxDrawBuffers; i ++)
@@ -241,6 +251,11 @@ _mesa_PushAttrib(GLbitfield mask)
struct gl_current_attrib *attr;
FLUSH_CURRENT( ctx, 0 );
attr = MALLOC_STRUCT( gl_current_attrib );
+  if (attr == NULL) {
+ _mesa_error(ctx, GL_OUT_OF_MEMORY, "glPushAttrib");
+ goto end;
+  }
+
memcpy( attr, &ctx->Current, sizeof(struct gl_current_attrib) );
save_attrib_data(&head, GL_CURRENT_BIT, attr);
 }
@@ -248,6 +263,11 @@ _mesa_PushAttrib(GLbitfield mask)
 if (mask & GL_DEPTH_BUFFER_BIT) {
struct gl_depthbuffer_attrib *attr;
attr = MALLOC_STRUCT( gl_depthbuffer_attrib );
+  if (attr == NULL) {
+ _mesa_error(ctx, GL_OUT_OF_MEMORY, "glPushAttrib");
+ goto end;
+  }
+
memcpy( attr, &ctx->Depth, sizeof(struct gl_depthbuffer_attrib) );
save_attrib_data(&head, GL_DEPTH_BUFFER_BIT, attr);
 }
@@ -256,6 +276,11 @@ _mesa_PushAttrib(GLbitfield mask)
struct gl_enable_attrib *attr;
GLuint i;
attr = MALLOC_STRUCT( gl_enable_attrib );
+  if (attr == NULL) {
+ _mesa_error(ctx, GL_OUT_OF_MEMORY, "glPushAttrib");
+ goto end;
+  }
+
/* Copy enable flags from all other attributes into the enable struct. 
*/
attr->AlphaTest = ctx->Color.AlphaEnabled;
attr->AutoNormal = ctx->Eval.AutoNormal;
@@ -331,6 +356,11 @@ _mesa_PushAttrib(GLbitfield mask)
 if (mask & GL_EVAL_BIT) {
struct gl_eval_attrib *attr;
attr = MALLOC_STRUCT( gl_eval_attrib );
+  if (attr == NULL) {
+ _mesa_error(ctx, GL_OUT_OF_MEMORY, "glPushAttrib");
+ goto end;
+  }
+
memcpy( attr, &ctx->Eval, sizeof(struct gl_eval_attrib) );
save_attrib_data(&head, GL_EVAL_BIT, attr);
 }
@@ -338,6 +368,11 @@ _mesa_PushAttrib(GLbitfield mask)
 if (mask & GL_FOG_BIT) {
struct gl_fog_attrib *attr;
attr = MALLOC_STRUCT( gl_fog_attrib );
+  if (attr == NULL) {
+ _mesa_error(ctx, GL_OUT_OF_MEMORY, "glPushAttrib");
+ goto end;
+  }
+
memcpy( attr, &ctx->Fog, sizeof(struct gl_fog_attrib) );
save_attrib_data(&head, GL_FOG_BIT, attr);
 }
@@ -345,6 +380,11 @@ _mesa_PushAttrib(GLbitfield mask)
 if (mask & GL_HINT_BIT) {
struct gl_hint_attrib *attr;
attr = MALLOC_STRUCT( gl_hint_attrib );
+  if (attr == NULL) {
+ _mesa_error(ctx, GL_OUT_OF_MEMORY, "glPushAttrib");
+ goto end;
+  }
+
memcpy( attr, &ctx->Hint, sizeof(struct gl_hint_attrib) );
save_attrib_data(&head, GL_HINT_BIT, attr);
 }
@@ -353,6 +393,11 @@ _mesa_PushAttrib(GLbitfield mask)
struct gl_light_attrib *attr;
FLUSH_CURRENT(ctx, 0);  /* flush material changes */
attr = MALLOC_STRUCT( gl_light_attrib );
+  if (attr == NULL) {
+ _mesa_error(ctx, GL_OUT_OF_MEMORY, "glPushAttrib");
+ goto end;
+  }
+
memcpy( attr, &ctx->Light, sizeof(struct gl_light_attrib) );
save_attrib_data(&head, GL_LIGHTING_BIT, attr);
 }
@@ -360,6 +405,11 @@ _mesa_PushAttrib(GLbitfield mask)
 if (mask & GL_LINE_BIT) {
struct gl_line_attrib *attr;
attr = MALLOC_STRUCT( gl_line_attrib );
+  if (attr == NULL) {
+ _mesa_error(ctx, GL_OUT_OF_MEMORY, "glPushAttrib");
+ goto end;
+  }
+
memcpy( attr, &ctx->Line, sizeof(struct gl_line_attrib) );
save_attrib_data(&head, GL_LINE_BIT, attr);
 }
@@ -367,6 +417,11 @@ _mesa_PushAttrib(GLbitfield mask)
 if (mask & GL_LIST_BIT) {
struct gl_list_attrib *attr;
a

Re: [Mesa-dev] [PATCH 2/3] mesa: Verify memory allocations success in _mesa_PushClientAttrib

[Ian Romanick]

On 12/02/2013 01:39 AM, Juha-Pekka Heikkila wrote:
> Check if any of the callocs fail and report it with _mesa_error
> if needed.
> 
> Signed-off-by: Juha-Pekka Heikkila 
> ---
>  src/mesa/main/attrib.c | 34 ++
>  1 file changed, 30 insertions(+), 4 deletions(-)
> 
> diff --git a/src/mesa/main/attrib.c b/src/mesa/main/attrib.c
> index c9332bd..2418fb0 100644
> --- a/src/mesa/main/attrib.c
> +++ b/src/mesa/main/attrib.c
> @@ -1488,6 +1488,12 @@ init_array_attrib_data(struct gl_context *ctx,
>  {
> /* Get a non driver gl_array_object. */
> attrib->ArrayObj = CALLOC_STRUCT( gl_array_object );
> +
> +   if (attrib->ArrayObj == NULL) {
> +  _mesa_error(ctx, GL_OUT_OF_MEMORY, "glPushClientAttrib");
> +  return;
> +   }
> +
> _mesa_initialize_array_object(ctx, attrib->ArrayObj, 0);
>  }
>  
> @@ -1516,7 +1522,7 @@ _mesa_PushClientAttrib(GLbitfield mask)
> GET_CURRENT_CONTEXT(ctx);
>  
> if (ctx->ClientAttribStackDepth >= MAX_CLIENT_ATTRIB_STACK_DEPTH) {
> -  _mesa_error( ctx, GL_STACK_OVERFLOW, "glPushClientAttrib" );
> +  _mesa_error(ctx, GL_STACK_OVERFLOW, "glPushClientAttrib");

In addition to Brian's comments, this is just spurious whitespace
changes.  Don't mix formatting chages with substantive changes.

>return;
> }
>  
> @@ -1529,10 +1535,19 @@ _mesa_PushClientAttrib(GLbitfield mask)
>struct gl_pixelstore_attrib *attr;
>/* packing attribs */
>attr = CALLOC_STRUCT( gl_pixelstore_attrib );
> +  if (attr == NULL) {
> + _mesa_error(ctx, GL_OUT_OF_MEMORY, "glPushClientAttrib");
> + goto end;
> +  }
>copy_pixelstore(ctx, attr, &ctx->Pack);
>save_attrib_data(&head, GL_CLIENT_PACK_BIT, attr);
>/* unpacking attribs */
>attr = CALLOC_STRUCT( gl_pixelstore_attrib );
> +  if (attr == NULL) {
> + _mesa_error(ctx, GL_OUT_OF_MEMORY, "glPushClientAttrib");
> + goto end;
> +  }
> +
>copy_pixelstore(ctx, attr, &ctx->Unpack);
>save_attrib_data(&head, GL_CLIENT_UNPACK_BIT, attr);
> }
> @@ -1540,13 +1555,24 @@ _mesa_PushClientAttrib(GLbitfield mask)
> if (mask & GL_CLIENT_VERTEX_ARRAY_BIT) {
>struct gl_array_attrib *attr;
>attr = CALLOC_STRUCT( gl_array_attrib );
> +  if (attr == NULL) {
> + _mesa_error(ctx, GL_OUT_OF_MEMORY, "glPushClientAttrib");
> + goto end;
> +  }
> +
>init_array_attrib_data(ctx, attr);
> +  if (attr->ArrayObj == NULL) {
> +  goto end;
> +  }
> +
>save_array_attrib(ctx, attr, &ctx->Array);
>save_attrib_data(&head, GL_CLIENT_VERTEX_ARRAY_BIT, attr);
> }
> -
> -   ctx->ClientAttribStack[ctx->ClientAttribStackDepth] = head;
> -   ctx->ClientAttribStackDepth++;
> +end:
> +   if (head != NULL) {
> +   ctx->ClientAttribStack[ctx->ClientAttribStackDepth] = head;
> +   ctx->ClientAttribStackDepth++;
> +   }
>  }
>  
>  
> 

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

Re: [Mesa-dev] [PATCH] glx: Add missing null check in gxl/dri2_glx.c

[Brian Paul]

On 12/02/2013 07:30 AM, Juha-Pekka Heikkila wrote:

Signed-off-by: Juha-Pekka Heikkila 
---
  src/glx/dri2_glx.c | 4 
  1 file changed, 4 insertions(+)

diff --git a/src/glx/dri2_glx.c b/src/glx/dri2_glx.c
index 3b33312..bfeebed 100644
--- a/src/glx/dri2_glx.c
+++ b/src/glx/dri2_glx.c
@@ -676,6 +676,10 @@ dri2FlushFrontBuffer(__DRIdrawable *driDrawable, void 
*loaderPrivate)
 psc = (struct dri2_screen *) pdraw->base.psc;

 priv = __glXInitialize(psc->base.dpy);
+
+   if (priv == NULL)
+   return;
+
 pdp = (struct dri2_display *) priv->dri2Display;
 gc = __glXGetCurrentContext();



Reviewed-by: Brian Paul 

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

Re: [Mesa-dev] [PATCH 3/3] mesa: Verify memory allocations success in _mesa_PushAttrib

[Ian Romanick]

On 12/02/2013 01:39 AM, Juha-Pekka Heikkila wrote:
> Check if any of the callocs fail and report it with _mesa_error
> if needed.
> 
> Signed-off-by: Juha-Pekka Heikkila 
> ---
>  src/mesa/main/attrib.c | 106 
> -
>  1 file changed, 104 insertions(+), 2 deletions(-)
> 
> diff --git a/src/mesa/main/attrib.c b/src/mesa/main/attrib.c
> index 2418fb0..037cd64 100644
> --- a/src/mesa/main/attrib.c
> +++ b/src/mesa/main/attrib.c
> @@ -222,6 +222,11 @@ _mesa_PushAttrib(GLbitfield mask)
> if (mask & GL_ACCUM_BUFFER_BIT) {
>struct gl_accum_attrib *attr;
>attr = MALLOC_STRUCT( gl_accum_attrib );
> +  if (attr == NULL) {
> + _mesa_error(ctx, GL_OUT_OF_MEMORY, "glPushAttrib");
> + goto end;
> +  }
> +
>memcpy( attr, &ctx->Accum, sizeof(struct gl_accum_attrib) );
>save_attrib_data(&head, GL_ACCUM_BUFFER_BIT, attr);

And what if save_attrib_data fails to allocate memory for head?

> }
> @@ -230,6 +235,11 @@ _mesa_PushAttrib(GLbitfield mask)
>GLuint i;
>struct gl_colorbuffer_attrib *attr;
>attr = MALLOC_STRUCT( gl_colorbuffer_attrib );
> +  if (attr == NULL) {
> + _mesa_error(ctx, GL_OUT_OF_MEMORY, "glPushAttrib");
> + goto end;
> +  }
> +
>memcpy( attr, &ctx->Color, sizeof(struct gl_colorbuffer_attrib) );
>/* push the Draw FBO's DrawBuffer[] state, not ctx->Color.DrawBuffer[] 
> */
>for (i = 0; i < ctx->Const.MaxDrawBuffers; i ++)
> @@ -241,6 +251,11 @@ _mesa_PushAttrib(GLbitfield mask)
>struct gl_current_attrib *attr;
>FLUSH_CURRENT( ctx, 0 );
>attr = MALLOC_STRUCT( gl_current_attrib );
> +  if (attr == NULL) {
> + _mesa_error(ctx, GL_OUT_OF_MEMORY, "glPushAttrib");
> + goto end;
> +  }
> +
>memcpy( attr, &ctx->Current, sizeof(struct gl_current_attrib) );
>save_attrib_data(&head, GL_CURRENT_BIT, attr);
> }
> @@ -248,6 +263,11 @@ _mesa_PushAttrib(GLbitfield mask)
> if (mask & GL_DEPTH_BUFFER_BIT) {
>struct gl_depthbuffer_attrib *attr;
>attr = MALLOC_STRUCT( gl_depthbuffer_attrib );
> +  if (attr == NULL) {
> + _mesa_error(ctx, GL_OUT_OF_MEMORY, "glPushAttrib");
> + goto end;
> +  }
> +
>memcpy( attr, &ctx->Depth, sizeof(struct gl_depthbuffer_attrib) );
>save_attrib_data(&head, GL_DEPTH_BUFFER_BIT, attr);
> }
> @@ -256,6 +276,11 @@ _mesa_PushAttrib(GLbitfield mask)
>struct gl_enable_attrib *attr;
>GLuint i;
>attr = MALLOC_STRUCT( gl_enable_attrib );
> +  if (attr == NULL) {
> + _mesa_error(ctx, GL_OUT_OF_MEMORY, "glPushAttrib");
> + goto end;
> +  }
> +
>/* Copy enable flags from all other attributes into the enable struct. 
> */
>attr->AlphaTest = ctx->Color.AlphaEnabled;
>attr->AutoNormal = ctx->Eval.AutoNormal;
> @@ -331,6 +356,11 @@ _mesa_PushAttrib(GLbitfield mask)
> if (mask & GL_EVAL_BIT) {
>struct gl_eval_attrib *attr;
>attr = MALLOC_STRUCT( gl_eval_attrib );
> +  if (attr == NULL) {
> + _mesa_error(ctx, GL_OUT_OF_MEMORY, "glPushAttrib");
> + goto end;
> +  }
> +
>memcpy( attr, &ctx->Eval, sizeof(struct gl_eval_attrib) );
>save_attrib_data(&head, GL_EVAL_BIT, attr);
> }
> @@ -338,6 +368,11 @@ _mesa_PushAttrib(GLbitfield mask)
> if (mask & GL_FOG_BIT) {
>struct gl_fog_attrib *attr;
>attr = MALLOC_STRUCT( gl_fog_attrib );
> +  if (attr == NULL) {
> + _mesa_error(ctx, GL_OUT_OF_MEMORY, "glPushAttrib");
> + goto end;
> +  }
> +
>memcpy( attr, &ctx->Fog, sizeof(struct gl_fog_attrib) );
>save_attrib_data(&head, GL_FOG_BIT, attr);
> }
> @@ -345,6 +380,11 @@ _mesa_PushAttrib(GLbitfield mask)
> if (mask & GL_HINT_BIT) {
>struct gl_hint_attrib *attr;
>attr = MALLOC_STRUCT( gl_hint_attrib );
> +  if (attr == NULL) {
> + _mesa_error(ctx, GL_OUT_OF_MEMORY, "glPushAttrib");
> + goto end;
> +  }
> +
>memcpy( attr, &ctx->Hint, sizeof(struct gl_hint_attrib) );
>save_attrib_data(&head, GL_HINT_BIT, attr);
> }
> @@ -353,6 +393,11 @@ _mesa_PushAttrib(GLbitfield mask)
>struct gl_light_attrib *attr;
>FLUSH_CURRENT(ctx, 0); /* flush material changes */
>attr = MALLOC_STRUCT( gl_light_attrib );
> +  if (attr == NULL) {
> + _mesa_error(ctx, GL_OUT_OF_MEMORY, "glPushAttrib");
> + goto end;
> +  }
> +
>memcpy( attr, &ctx->Light, sizeof(struct gl_light_attrib) );
>save_attrib_data(&head, GL_LIGHTING_BIT, attr);
> }
> @@ -360,6 +405,11 @@ _mesa_PushAttrib(GLbitfield mask)
> if (mask & GL_LINE_BIT) {
>struct gl_line_attrib *attr;
>attr = MALLOC_STRUCT( gl_line_attrib );
> +  if (attr == NULL) {
> + _mesa_error(ctx, GL_OUT_OF_MEMORY, "glPushAttrib");
> + goto end;
> + 

Re: [Mesa-dev] [PATCH 8/8] mesa: Remove GL_MESA_texture_array cruft from gl.h

[Brian Paul]

On 11/26/2013 04:54 PM, Ian Romanick wrote:

From: Ian Romanick 

glext.h has had all the necessary bits for years.

Signed-off-by: Ian Romanick 
---
  include/GL/gl.h | 33 -
  1 file changed, 33 deletions(-)



For the series:
Reviewed-by: Brian Paul 

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

Re: [Mesa-dev] [Mesa-stable] [PATCH v3] egl: add HAVE_LIBDRM define, fix EGL X11 platform

[Chad Versace]

On 12/02/2013 09:55 PM, Tapani Pälli wrote:

On 12/02/2013 11:06 PM, Chad Versace wrote:

On 12/01/2013 01:53 AM, Tapani Pälli wrote:

Commit a594cec broke EGL X11 backend by adding dependency between
X11 and DRM backends requiring HAVE_EGL_PLATFORM_DRM defined for X11.

This patch fixes the issue by adding additional define for libdrm
detection independent of which backend is being compiled. Tested by
compiling Mesa with '--with-egl-platforms=x11' and running es2gears_x11
+ glbenchmark2.7 successfully.

v2: return true for dri2_auth if running without libdrm (Samuel)
v3: check libdrm when building EGL drm platform + AM_CFLAGS fix (Emil)

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=72062
Signed-off-by: Tapani Pälli 
Reviewed-by: Emil Velikov 
Cc: Samuel Thibault 
Cc: mesa-sta...@lists.freedesktop.org



I tested this patch with --with-egl-platforms=x11, and it works
for me.

But, why return true from dri2_authenticate() when building
without libdrm? I expected dri2_authenticate() to return false
in that case, because the function never authenticates with the
Xserver without drmGetMagic().



I hope Samuel is able to answer this. I'm not sure which platform/driver uses 
this configuration, I was trying to ask
for it also in the bug to be able to test this. I can also leave that change 
out for now and return FALSE by default if
no conclusion to this.


> I'd really like a working EGL ...

A working EGL is better than a slightly buggy EGL, so I've committed the patch 
to master.

Samuel, please explain why you requested that dri2_authenticate() return true 
when HAVE_LIBDRM == false.
Are you using some arcane build configuration that requires this?

I want it to return false because, without libdrm, the function never 
authenticates with the Xserver.
No authentication occurs, so the function should return false.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 6/9] mesa: consolidate multiple next_mipmap_level_size

[Brian Paul]

On 11/25/2013 05:47 PM, Courtney Goeltzenleuchter wrote:

Refactor to make next_mipmap_level_size defined in mipmap.c a
_mesa_ helper function that can then be used by texture_view

Signed-off-by: Courtney Goeltzenleuchter 
---
  src/mesa/main/mipmap.c |  8 
  src/mesa/main/mipmap.h |  4 
  src/mesa/main/texstorage.c | 25 +++--
  3 files changed, 11 insertions(+), 26 deletions(-)

diff --git a/src/mesa/main/mipmap.c b/src/mesa/main/mipmap.c
index 180f891..0330157 100644
--- a/src/mesa/main/mipmap.c
+++ b/src/mesa/main/mipmap.c
@@ -1767,8 +1767,8 @@ _mesa_generate_mipmap_level(GLenum target,
   * compute next (level+1) image size
   * \return GL_FALSE if no smaller size can be generated (eg. src is 1x1x1 
size)
   */
-static GLboolean
-next_mipmap_level_size(GLenum target, GLint border,
+GLboolean
+_mesa_next_mipmap_level_size(GLenum target, GLint border,
 GLint srcWidth, GLint srcHeight, GLint srcDepth,
 GLint *dstWidth, GLint *dstHeight, GLint *dstDepth)
  {
@@ -1911,7 +1911,7 @@ generate_mipmap_uncompressed(struct gl_context *ctx, 
GLenum target,
srcDepth = srcImage->Depth;
border = srcImage->Border;

-  nextLevel = next_mipmap_level_size(target, border,
+  nextLevel = _mesa_next_mipmap_level_size(target, border,
   srcWidth, srcHeight, srcDepth,
   &dstWidth, &dstHeight, &dstDepth);
if (!nextLevel)
@@ -2102,7 +2102,7 @@ generate_mipmap_compressed(struct gl_context *ctx, GLenum 
target,
srcDepth = srcImage->Depth;
border = srcImage->Border;

-  nextLevel = next_mipmap_level_size(target, border,
+  nextLevel = _mesa_next_mipmap_level_size(target, border,
   srcWidth, srcHeight, srcDepth,
   &dstWidth, &dstHeight, &dstDepth);
if (!nextLevel)
diff --git a/src/mesa/main/mipmap.h b/src/mesa/main/mipmap.h
index d5bd1d8..ee91df0 100644
--- a/src/mesa/main/mipmap.h
+++ b/src/mesa/main/mipmap.h
@@ -51,5 +51,9 @@ extern void
  _mesa_generate_mipmap(struct gl_context *ctx, GLenum target,
struct gl_texture_object *texObj);

+extern GLboolean
+_mesa_next_mipmap_level_size(GLenum target, GLint border,
+   GLint srcWidth, GLint srcHeight, GLint srcDepth,
+   GLint *dstWidth, GLint *dstHeight, GLint *dstDepth);

  #endif /* MIPMAP_H */
diff --git a/src/mesa/main/texstorage.c b/src/mesa/main/texstorage.c
index 84b8f82..8d472a2 100644
--- a/src/mesa/main/texstorage.c
+++ b/src/mesa/main/texstorage.c
@@ -37,6 +37,7 @@
  #include "macros.h"
  #include "teximage.h"
  #include "texobj.h"
+#include "mipmap.h"
  #include "texstorage.h"
  #include "mtypes.h"

@@ -100,27 +101,6 @@ legal_texobj_target(struct gl_context *ctx, GLuint dims, 
GLenum target)
  }


-/**
- * Compute the size of the next mipmap level.
- */
-static void
-next_mipmap_level_size(GLenum target,
-   GLint *width, GLint *height, GLint *depth)
-{
-   if (*width > 1) {
-  *width /= 2;
-   }
-
-   if ((*height > 1) && (target != GL_TEXTURE_1D_ARRAY)) {
-  *height /= 2;
-   }
-
-   if ((*depth > 1) && (target != GL_TEXTURE_2D_ARRAY)) {
-  *depth /= 2;
-   }
-}
-
-
  /** Helper to get a particular texture image in a texture object */
  static struct gl_texture_image *
  get_tex_image(struct gl_context *ctx,
@@ -164,7 +144,8 @@ initialize_texture_fields(struct gl_context *ctx,
  0, internalFormat, texFormat);
}

-  next_mipmap_level_size(target, &levelWidth, &levelHeight, &levelDepth);
+  _mesa_next_mipmap_level_size(target, 0, levelWidth, levelHeight, 
levelDepth,
+   &levelWidth, &levelHeight, &levelDepth);
 }
 return GL_TRUE;
  }



Reviewed-by: Brian Paul 

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

Re: [Mesa-dev] [PATCH 7/9] mesa: Fill out ARB_texture_view entry points

[Brian Paul]

This looks pretty good.  Just a bunch of nitpicks below.


On 11/25/2013 05:49 PM, Courtney Goeltzenleuchter wrote:

Add Mesa TextureView logic.
Incorporate feedback on ARB_texture_view:
- Add S3TC VIEW_CLASSes to compatibility table
- Use existing _mesa_get_tex_image
- Clean up error strings
- Use bool instead of GLboolean for internal functions
- Split compound level & layer test into individual tests

Signed-off-by: Courtney Goeltzenleuchter 
---
  src/mesa/main/textureview.c | 540 +++-
  1 file changed, 539 insertions(+), 1 deletion(-)

diff --git a/src/mesa/main/textureview.c b/src/mesa/main/textureview.c
index 4a6bd62..1bfc86b 100644
--- a/src/mesa/main/textureview.c
+++ b/src/mesa/main/textureview.c
@@ -38,10 +38,318 @@
  #include "macros.h"
  #include "teximage.h"
  #include "texobj.h"
+#include "mipmap.h"
  #include "texstorage.h"
  #include "textureview.h"
+#include "stdbool.h"
  #include "mtypes.h"

+/* Table 3.X.2 (Compatible internal formats for TextureView)
+---
+| Class | Internal formats|
+---
+| VIEW_CLASS_128_BITS   | RGBA32F, RGBA32UI, RGBA32I  |
+---
+| VIEW_CLASS_96_BITS| RGB32F, RGB32UI, RGB32I |
+---
+| VIEW_CLASS_64_BITS| RGBA16F, RG32F, RGBA16UI, RG32UI, RGBA16I,  |
+|   | RG32I, RGBA16, RGBA16_SNORM |
+---
+| VIEW_CLASS_48_BITS| RGB16, RGB16_SNORM, RGB16F, RGB16UI, RGB16I |
+---
+| VIEW_CLASS_32_BITS| RG16F, R11F_G11F_B10F, R32F,|
+|   | RGB10_A2UI, RGBA8UI, RG16UI, R32UI, |
+|   | RGBA8I, RG16I, R32I, RGB10_A2, RGBA8, RG16, |
+|   | RGBA8_SNORM, RG16_SNORM, SRGB8_ALPHA8, RGB9_E5  |
+---
+| VIEW_CLASS_24_BITS| RGB8, RGB8_SNORM, SRGB8, RGB8UI, RGB8I  |
+---
+| VIEW_CLASS_16_BITS| R16F, RG8UI, R16UI, RG8I, R16I, RG8, R16,   |
+|   | RG8_SNORM, R16_SNORM|
+---
+| VIEW_CLASS_8_BITS | R8UI, R8I, R8, R8_SNORM |
+---
+| VIEW_CLASS_RGTC1_RED  | COMPRESSED_RED_RGTC1,   |
+|   | COMPRESSED_SIGNED_RED_RGTC1 |
+---
+| VIEW_CLASS_RGTC2_RG   | COMPRESSED_RG_RGTC2,|
+|   | COMPRESSED_SIGNED_RG_RGTC2  |
+---
+| VIEW_CLASS_BPTC_UNORM | COMPRESSED_RGBA_BPTC_UNORM, |
+|   | COMPRESSED_SRGB_ALPHA_BPTC_UNORM|
+---
+| VIEW_CLASS_BPTC_FLOAT | COMPRESSED_RGB_BPTC_SIGNED_FLOAT,   |
+|   | COMPRESSED_RGB_BPTC_UNSIGNED_FLOAT  |
+---
+ */
+struct internal_format_class_info {
+   GLenum view_class;
+   GLenum internal_format;
+};
+#define INFO(c,f) {GL_##c, GL_##f}


I'd like to see this INFO() macro stuff go away.  It just obfuscates the 
code.




+static const struct internal_format_class_info compatible_internal_formats[] = 
{
+   INFO(VIEW_CLASS_128_BITS, RGBA32F),
+   INFO(VIEW_CLASS_128_BITS, RGBA32UI),
+   INFO(VIEW_CLASS_128_BITS, RGBA32I),
+   INFO(VIEW_CLASS_96_BITS, RGB32F),
+   INFO(VIEW_CLASS_96_BITS, RGB32UI),
+   INFO(VIEW_CLASS_96_BITS, RGB32I),
+   INFO(VIEW_CLASS_64_BITS, RGBA16F),
+   INFO(VIEW_CLASS_64_BITS, RG32F),
+   INFO(VIEW_CLASS_64_BITS, RGBA16UI),
+   INFO(VIEW_CLASS_64_BITS, RG32UI),
+   INFO(VIEW_CLASS_64_BITS, RGBA16I),
+   INFO(VIEW_CLASS_64_BITS, RG32I),
+   INFO(VIEW_CLASS_64_BITS, RGBA16),
+   INFO(VIEW_CLASS_64_BITS, RGBA16_SNORM),
+   INFO(VIEW_CLASS_48_BITS, RGB16),
+   INFO(VIEW_CLASS_48_BITS, RGB16_SNORM),
+   INFO(VIEW_CLASS_48_BITS, RGB16F),
+   INFO(VIEW_CLASS_48_BITS, RGB16UI),
+   INFO(VIEW_CLASS_48_BITS, RGB16I),
+   INFO(VI

[Mesa-dev] [PATCH] glxinfo: print ES2 extension list

[Marek Olšák]

From: Marek Olšák 

---
 src/xdemos/glxinfo.c | 43 ++-
 1 file changed, 30 insertions(+), 13 deletions(-)

diff --git a/src/xdemos/glxinfo.c b/src/xdemos/glxinfo.c
index c9edc6a..a008ea8 100644
--- a/src/xdemos/glxinfo.c
+++ b/src/xdemos/glxinfo.c
@@ -760,7 +760,7 @@ create_context_flags(Display *dpy, GLXFBConfig fbconfig, 
int major, int minor,
  */
 static GLXContext
 create_context_with_config(Display *dpy, GLXFBConfig config,
-   Bool coreProfile, Bool direct)
+   Bool coreProfile, Bool es2Profile, Bool direct)
 {
GLXContext ctx = 0;
 
@@ -787,6 +787,21 @@ create_context_with_config(Display *dpy, GLXFBConfig 
config,
   return 0;
}
 
+   if (es2Profile) {
+#ifdef GLX_CONTEXT_ES2_PROFILE_BIT_EXT
+  if (extension_supported("GLX_EXT_create_context_es2_profile",
+  glXQueryExtensionsString(dpy, 0))) {
+ ctx = create_context_flags(dpy, config, 2, 0, 0x0,
+GLX_CONTEXT_ES2_PROFILE_BIT_EXT,
+direct);
+ return ctx;
+  }
+  return 0;
+#else
+  return 0;
+#endif
+   }
+
/* GLX should return a context of the latest GL version that supports
 * the full profile.
 */
@@ -832,8 +847,8 @@ choose_xvisinfo(Display *dpy, int scrnum)
 
 static Bool
 print_screen_info(Display *dpy, int scrnum, Bool allowDirect,
-  Bool coreProfile, Bool limits, Bool singleLine,
-  Bool coreWorked)
+  Bool coreProfile, Bool es2Profile, Bool limits,
+ Bool singleLine, Bool coreWorked)
 {
Window win;
XSetWindowAttributes attr;
@@ -843,7 +858,8 @@ print_screen_info(Display *dpy, int scrnum, Bool 
allowDirect,
XVisualInfo *visinfo;
int width = 100, height = 100;
GLXFBConfig *fbconfigs;
-   const char *oglstring = coreProfile ? "OpenGL core profile" : "OpenGL";
+   const char *oglstring = coreProfile ? "OpenGL core profile" :
+  es2Profile ? "OpenGL ES profile" : "OpenGL";
 
root = RootWindow(dpy, scrnum);
 
@@ -853,30 +869,30 @@ print_screen_info(Display *dpy, int scrnum, Bool 
allowDirect,
fbconfigs = choose_fb_config(dpy, scrnum);
if (fbconfigs) {
   ctx = create_context_with_config(dpy, fbconfigs[0],
-   coreProfile, allowDirect);
+   coreProfile, es2Profile, allowDirect);
   if (!ctx && allowDirect && !coreProfile) {
  /* try indirect */
  ctx = create_context_with_config(dpy, fbconfigs[0],
-  coreProfile, False);
+  coreProfile, es2Profile, False);
   }
 
   visinfo = glXGetVisualFromFBConfig(dpy, fbconfigs[0]);
   XFree(fbconfigs);
}
-   else if (!coreProfile) {
+   else if (!coreProfile && !es2Profile) {
   visinfo = choose_xvisinfo(dpy, scrnum);
   if (visinfo)
 ctx = glXCreateContext(dpy, visinfo, NULL, allowDirect);
} else
   visinfo = NULL;
 
-   if (!visinfo && !coreProfile) {
+   if (!visinfo && !coreProfile && !es2Profile) {
   fprintf(stderr, "Error: couldn't find RGB GLX visual or fbconfig\n");
   return False;
}
 
if (!ctx) {
-  if (!coreProfile)
+  if (!coreProfile && !es2Profile)
 fprintf(stderr, "Error: glXCreateContext failed\n");
   XFree(visinfo);
   return False;
@@ -990,7 +1006,7 @@ print_screen_info(Display *dpy, int scrnum, Bool 
allowDirect,
 #endif
   CheckError(__LINE__);
 #ifdef GL_VERSION_3_0
-  if (version >= 30) {
+  if (version >= 30 && !es2Profile) {
  GLint flags;
  glGetIntegerv(GL_CONTEXT_FLAGS, &flags);
  printf("%s context flags: %s\n", oglstring, 
context_flags_string(flags));
@@ -998,7 +1014,7 @@ print_screen_info(Display *dpy, int scrnum, Bool 
allowDirect,
 #endif
   CheckError(__LINE__);
 #ifdef GL_VERSION_3_2
-  if (version >= 32) {
+  if (version >= 32 && !es2Profile) {
  GLint mask;
  glGetIntegerv(GL_CONTEXT_PROFILE_MASK, &mask);
  printf("%s profile mask: %s\n", oglstring, profile_mask_string(mask));
@@ -1730,8 +1746,9 @@ main(int argc, char *argv[])
   print_display_info(dpy);
   for (scrnum = 0; scrnum < numScreens; scrnum++) {
  mesa_hack(dpy, scrnum);
- coreWorked = print_screen_info(dpy, scrnum, allowDirect, True, 
limits, singleLine, False);
- print_screen_info(dpy, scrnum, allowDirect, False, limits, 
singleLine, coreWorked);
+ coreWorked = print_screen_info(dpy, scrnum, allowDirect, True, False, 
limits, singleLine, False);
+ print_screen_info(dpy, scrnum, allowDirect, False, False, limits, 
singleLine, coreWorked);
+print_screen_info(dpy, scrnum, allowDirect, False, True, False, 
singleLine, True);
 
  printf("\n");
  print_visual_info(d

Re: [Mesa-dev] [PATCH] glxinfo: print ES2 extension list

[Brian Paul]

On 12/03/2013 10:58 AM, Marek Olšák wrote:

From: Marek Olšák 

---
  src/xdemos/glxinfo.c | 43 ++-
  1 file changed, 30 insertions(+), 13 deletions(-)

diff --git a/src/xdemos/glxinfo.c b/src/xdemos/glxinfo.c
index c9edc6a..a008ea8 100644
--- a/src/xdemos/glxinfo.c
+++ b/src/xdemos/glxinfo.c
@@ -760,7 +760,7 @@ create_context_flags(Display *dpy, GLXFBConfig fbconfig, 
int major, int minor,
   */
  static GLXContext
  create_context_with_config(Display *dpy, GLXFBConfig config,
-   Bool coreProfile, Bool direct)
+   Bool coreProfile, Bool es2Profile, Bool direct)
  {
 GLXContext ctx = 0;

@@ -787,6 +787,21 @@ create_context_with_config(Display *dpy, GLXFBConfig 
config,
return 0;
 }

+   if (es2Profile) {
+#ifdef GLX_CONTEXT_ES2_PROFILE_BIT_EXT
+  if (extension_supported("GLX_EXT_create_context_es2_profile",
+  glXQueryExtensionsString(dpy, 0))) {
+ ctx = create_context_flags(dpy, config, 2, 0, 0x0,
+GLX_CONTEXT_ES2_PROFILE_BIT_EXT,
+direct);
+ return ctx;
+  }
+  return 0;
+#else
+  return 0;
+#endif
+   }
+
 /* GLX should return a context of the latest GL version that supports
  * the full profile.
  */
@@ -832,8 +847,8 @@ choose_xvisinfo(Display *dpy, int scrnum)

  static Bool
  print_screen_info(Display *dpy, int scrnum, Bool allowDirect,
-  Bool coreProfile, Bool limits, Bool singleLine,
-  Bool coreWorked)
+  Bool coreProfile, Bool es2Profile, Bool limits,
+ Bool singleLine, Bool coreWorked)
  {
 Window win;
 XSetWindowAttributes attr;
@@ -843,7 +858,8 @@ print_screen_info(Display *dpy, int scrnum, Bool 
allowDirect,
 XVisualInfo *visinfo;
 int width = 100, height = 100;
 GLXFBConfig *fbconfigs;
-   const char *oglstring = coreProfile ? "OpenGL core profile" : "OpenGL";
+   const char *oglstring = coreProfile ? "OpenGL core profile" :
+  es2Profile ? "OpenGL ES profile" : "OpenGL";

 root = RootWindow(dpy, scrnum);

@@ -853,30 +869,30 @@ print_screen_info(Display *dpy, int scrnum, Bool 
allowDirect,
 fbconfigs = choose_fb_config(dpy, scrnum);
 if (fbconfigs) {
ctx = create_context_with_config(dpy, fbconfigs[0],
-   coreProfile, allowDirect);
+   coreProfile, es2Profile, allowDirect);
if (!ctx && allowDirect && !coreProfile) {
   /* try indirect */
   ctx = create_context_with_config(dpy, fbconfigs[0],
-  coreProfile, False);
+  coreProfile, es2Profile, False);
}

visinfo = glXGetVisualFromFBConfig(dpy, fbconfigs[0]);
XFree(fbconfigs);
 }
-   else if (!coreProfile) {
+   else if (!coreProfile && !es2Profile) {
visinfo = choose_xvisinfo(dpy, scrnum);
if (visinfo)
 ctx = glXCreateContext(dpy, visinfo, NULL, allowDirect);
 } else
visinfo = NULL;

-   if (!visinfo && !coreProfile) {
+   if (!visinfo && !coreProfile && !es2Profile) {
fprintf(stderr, "Error: couldn't find RGB GLX visual or fbconfig\n");
return False;
 }

 if (!ctx) {
-  if (!coreProfile)
+  if (!coreProfile && !es2Profile)
 fprintf(stderr, "Error: glXCreateContext failed\n");
XFree(visinfo);
return False;
@@ -990,7 +1006,7 @@ print_screen_info(Display *dpy, int scrnum, Bool 
allowDirect,
  #endif
CheckError(__LINE__);
  #ifdef GL_VERSION_3_0
-  if (version >= 30) {
+  if (version >= 30 && !es2Profile) {
   GLint flags;
   glGetIntegerv(GL_CONTEXT_FLAGS, &flags);
   printf("%s context flags: %s\n", oglstring, 
context_flags_string(flags));
@@ -998,7 +1014,7 @@ print_screen_info(Display *dpy, int scrnum, Bool 
allowDirect,
  #endif
CheckError(__LINE__);
  #ifdef GL_VERSION_3_2
-  if (version >= 32) {
+  if (version >= 32 && !es2Profile) {
   GLint mask;
   glGetIntegerv(GL_CONTEXT_PROFILE_MASK, &mask);
   printf("%s profile mask: %s\n", oglstring, 
profile_mask_string(mask));
@@ -1730,8 +1746,9 @@ main(int argc, char *argv[])
print_display_info(dpy);
for (scrnum = 0; scrnum < numScreens; scrnum++) {
   mesa_hack(dpy, scrnum);
- coreWorked = print_screen_info(dpy, scrnum, allowDirect, True, 
limits, singleLine, False);
- print_screen_info(dpy, scrnum, allowDirect, False, limits, 
singleLine, coreWorked);
+ coreWorked = print_screen_info(dpy, scrnum, allowDirect, True, False, 
limits, singleLine, False);
+ print_screen_info(dpy, scrnum, allowDirect, False, False, limits, 
singleLine, coreWorked);
+print_screen_info(dpy, scrnum, allowDirect, F

Re: [Mesa-dev] [PATCH 06/23] i965: Define common register base class shared between both back-ends.

[Francisco Jerez]

Francisco Jerez  writes:

> Petri Latvala  writes:
>[...]
>> Not saying it kosher either way though, calling assignment operator in a 
>> constructor is iffy :P. The proper way is selecting the correct base 
>> class constructor to call.
>>
Again, that probably belongs to a separate clean-up commit...  See
attachment.

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

From e1660cb525ced30db38dd2e373da7bf62235c132 Mon Sep 17 00:00:00 2001
From: Francisco Jerez 
Date: Tue, 3 Dec 2013 09:53:31 -0800
Subject: [PATCH] i965: Delegate common register initialization to the
 backend_reg constructors.

And stop bashing the register structure with zeroes on ::init().
---
 src/mesa/drivers/dri/i965/brw_fs.cpp   | 58 +++
 src/mesa/drivers/dri/i965/brw_fs.h |  5 +-
 src/mesa/drivers/dri/i965/brw_shader.cpp   | 38 +++-
 src/mesa/drivers/dri/i965/brw_shader.h |  4 ++
 src/mesa/drivers/dri/i965/brw_vec4.cpp | 80 ++
 src/mesa/drivers/dri/i965/brw_vec4.h   | 10 ++--
 src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp | 20 +++
 7 files changed, 107 insertions(+), 108 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp b/src/mesa/drivers/dri/i965/brw_fs.cpp
index ca152d1..f5d85ad 100644
--- a/src/mesa/drivers/dri/i965/brw_fs.cpp
+++ b/src/mesa/drivers/dri/i965/brw_fs.cpp
@@ -378,57 +378,51 @@ fs_visitor::can_do_source_mods(fs_inst *inst)
 void
 fs_reg::init()
 {
-   memset(this, 0, sizeof(*this));
+   negate = false;
+   abs = false;
+   subreg_offset = 0;
stride = 1;
+   reladdr = NULL;
 }
 
 /** Generic unset register constructor. */
 fs_reg::fs_reg()
 {
init();
-   this->file = BAD_FILE;
 }
 
 /** Immediate value constructor. */
-fs_reg::fs_reg(float f)
+fs_reg::fs_reg(float f) :
+   backend_reg(f)
 {
init();
-   this->file = IMM;
-   this->type = BRW_REGISTER_TYPE_F;
-   this->imm.f = f;
 }
 
 /** Immediate value constructor. */
-fs_reg::fs_reg(int32_t i)
+fs_reg::fs_reg(int32_t i) :
+   backend_reg(i)
 {
init();
-   this->file = IMM;
-   this->type = BRW_REGISTER_TYPE_D;
-   this->imm.i = i;
 }
 
 /** Immediate value constructor. */
-fs_reg::fs_reg(uint32_t u)
+fs_reg::fs_reg(uint32_t u) :
+   backend_reg(u)
 {
init();
-   this->file = IMM;
-   this->type = BRW_REGISTER_TYPE_UD;
-   this->imm.u = u;
 }
 
 /** Fixed brw_reg. */
-fs_reg::fs_reg(struct brw_reg fixed_hw_reg)
+fs_reg::fs_reg(struct brw_reg fixed_hw_reg) :
+   backend_reg(fixed_hw_reg)
 {
init();
-   this->file = HW_REG;
-   this->fixed_hw_reg = fixed_hw_reg;
-   this->type = fixed_hw_reg.type;
 }
 
-fs_reg::fs_reg(const backend_reg ®)
+fs_reg::fs_reg(const backend_reg ®) :
+   backend_reg(reg)
 {
init();
-   *static_cast(this) = reg;
 }
 
 bool
@@ -811,33 +805,23 @@ fs_visitor::virtual_grf_alloc(int size)
return virtual_grf_count++;
 }
 
-/** Fixed HW reg constructor. */
-fs_reg::fs_reg(enum register_file file, int reg)
+fs_reg::fs_reg(enum register_file file, int reg) :
+   backend_reg(file, reg, BRW_REGISTER_TYPE_F)
 {
init();
-   this->file = file;
-   this->reg = reg;
-   this->type = BRW_REGISTER_TYPE_F;
 }
 
-/** Fixed HW reg constructor. */
-fs_reg::fs_reg(enum register_file file, int reg, uint32_t type)
+fs_reg::fs_reg(enum register_file file, int reg, uint32_t type) :
+   backend_reg(file, reg, type)
 {
init();
-   this->file = file;
-   this->reg = reg;
-   this->type = type;
 }
 
-/** Automatic reg constructor. */
-fs_reg::fs_reg(class fs_visitor *v, const struct glsl_type *type)
+fs_reg::fs_reg(class fs_visitor *v, const struct glsl_type *type) :
+   backend_reg(GRF, v->virtual_grf_alloc(v->type_size(type)),
+   brw_type_for_base_type(type))
 {
init();
-
-   this->file = GRF;
-   this->reg = v->virtual_grf_alloc(v->type_size(type));
-   this->reg_offset = 0;
-   this->type = brw_type_for_base_type(type);
 }
 
 fs_reg *
diff --git a/src/mesa/drivers/dri/i965/brw_fs.h b/src/mesa/drivers/dri/i965/brw_fs.h
index ae4a6f5..7a9b727 100644
--- a/src/mesa/drivers/dri/i965/brw_fs.h
+++ b/src/mesa/drivers/dri/i965/brw_fs.h
@@ -63,8 +63,6 @@ class fs_reg : public backend_reg {
 public:
DECLARE_RALLOC_CXX_OPERATORS(fs_reg)
 
-   void init();
-
fs_reg();
fs_reg(float f);
fs_reg(int32_t i);
@@ -90,6 +88,9 @@ public:
int stride; /**< Register region horizontal stride */
 
fs_reg *reladdr;
+
+private:
+   void init();
 };
 
 static inline fs_reg
diff --git a/src/mesa/drivers/dri/i965/brw_shader.cpp b/src/mesa/drivers/dri/i965/brw_shader.cpp
index d903bb5..7667c50 100644
--- a/src/mesa/drivers/dri/i965/brw_shader.cpp
+++ b/src/mesa/drivers/dri/i965/brw_shader.cpp
@@ -571,12 +571,48 @@ backend_reg::backend_reg() :
 backend_reg::backend_reg(struct brw_reg fixed_hw_reg) :
file(HW_REG),
reg(0), reg_offset(0),
-   type(BRW_REGISTER_

Re: [Mesa-dev] [PATCH] Modified _mesa_align_free to have consistent behavior when dealing with NULL memory address.

[Brian Paul]

On 11/26/2013 10:59 PM, Siavash Eliasi wrote:

---
  src/mesa/main/imports.c|  7 +--
  src/mesa/math/m_matrix.c   | 13 +
  src/mesa/program/prog_parameter.c  |  3 +--
  src/mesa/state_tracker/st_cb_texture.c |  6 ++
  src/mesa/swrast/s_texture.c|  7 +++
  src/mesa/tnl/t_vertex.c|  6 ++
  src/mesa/vbo/vbo_exec_api.c|  9 -
  7 files changed, 22 insertions(+), 29 deletions(-)

diff --git a/src/mesa/main/imports.c b/src/mesa/main/imports.c
index 277e947..edfc0d1 100644
--- a/src/mesa/main/imports.c
+++ b/src/mesa/main/imports.c
@@ -177,6 +177,9 @@ _mesa_align_free(void *ptr)
  #elif defined(_WIN32) && defined(_MSC_VER)
 _aligned_free(ptr);
  #else
+   if (ptr == NULL)
+  return;
+


We can't have code before declarations like this (MSVC will error and 
some other compilers warn depending on the C variant being targeted.)


So, how about:

   if (ptr) {
  void **cubbyHole = (void **) ((char *) ptr - sizeof(void *));
  void *realAddr = *cubbyHole;
  free(realAddr);
   }

Maybe update the comment on the function to say that passing a NULL 
pointer is legal.


The rest looks fine.

With that fixed: Reviewed-by: Brian Paul 

Do you need someone to push patches for you?



 void **cubbyHole = (void **) ((char *) ptr - sizeof(void *));
 void *realAddr = *cubbyHole;
 free(realAddr);
@@ -199,8 +202,8 @@ _mesa_align_realloc(void *oldBuffer, size_t oldSize, size_t 
newSize,
 if (newBuf && oldBuffer && copySize > 0) {
memcpy(newBuf, oldBuffer, copySize);
 }
-   if (oldBuffer)
-  _mesa_align_free(oldBuffer);
+
+   _mesa_align_free(oldBuffer);
 return newBuf;
  #endif
  }
diff --git a/src/mesa/math/m_matrix.c b/src/mesa/math/m_matrix.c
index 2902315..274f969 100644
--- a/src/mesa/math/m_matrix.c
+++ b/src/mesa/math/m_matrix.c
@@ -1488,14 +1488,11 @@ _math_matrix_ctr( GLmatrix *m )
  void
  _math_matrix_dtr( GLmatrix *m )
  {
-   if (m->m) {
-  _mesa_align_free( m->m );
-  m->m = NULL;
-   }
-   if (m->inv) {
-  _mesa_align_free( m->inv );
-  m->inv = NULL;
-   }
+   _mesa_align_free( m->m );
+   m->m = NULL;
+
+   _mesa_align_free( m->inv );
+   m->inv = NULL;
  }

  /*@}*/
diff --git a/src/mesa/program/prog_parameter.c 
b/src/mesa/program/prog_parameter.c
index 4d9cf08..54531d2 100644
--- a/src/mesa/program/prog_parameter.c
+++ b/src/mesa/program/prog_parameter.c
@@ -83,8 +83,7 @@ _mesa_free_parameter_list(struct gl_program_parameter_list 
*paramList)
free((void *)paramList->Parameters[i].Name);
 }
 free(paramList->Parameters);
-   if (paramList->ParameterValues)
-  _mesa_align_free(paramList->ParameterValues);
+   _mesa_align_free(paramList->ParameterValues);
 free(paramList);
  }

diff --git a/src/mesa/state_tracker/st_cb_texture.c 
b/src/mesa/state_tracker/st_cb_texture.c
index faa9ee3..f33d3cf 100644
--- a/src/mesa/state_tracker/st_cb_texture.c
+++ b/src/mesa/state_tracker/st_cb_texture.c
@@ -175,10 +175,8 @@ st_FreeTextureImageBuffer(struct gl_context *ctx,
pipe_resource_reference(&stImage->pt, NULL);
 }

-   if (stImage->TexData) {
-  _mesa_align_free(stImage->TexData);
-  stImage->TexData = NULL;
-   }
+   _mesa_align_free(stImage->TexData);
+   stImage->TexData = NULL;
  }


diff --git a/src/mesa/swrast/s_texture.c b/src/mesa/swrast/s_texture.c
index 27803c5..c08a4e9 100644
--- a/src/mesa/swrast/s_texture.c
+++ b/src/mesa/swrast/s_texture.c
@@ -164,10 +164,9 @@ _swrast_free_texture_image_buffer(struct gl_context *ctx,
struct gl_texture_image *texImage)
  {
 struct swrast_texture_image *swImage = swrast_texture_image(texImage);
-   if (swImage->Buffer) {
-  _mesa_align_free(swImage->Buffer);
-  swImage->Buffer = NULL;
-   }
+
+   _mesa_align_free(swImage->Buffer);
+   swImage->Buffer = NULL;

 free(swImage->ImageSlices);
 swImage->ImageSlices = NULL;
diff --git a/src/mesa/tnl/t_vertex.c b/src/mesa/tnl/t_vertex.c
index c7a745e..8c4195e 100644
--- a/src/mesa/tnl/t_vertex.c
+++ b/src/mesa/tnl/t_vertex.c
@@ -546,10 +546,8 @@ void _tnl_free_vertices( struct gl_context *ctx )
struct tnl_clipspace *vtx = GET_VERTEX_STATE(ctx);
struct tnl_clipspace_fastpath *fp, *tmp;

-  if (vtx->vertex_buf) {
- _mesa_align_free(vtx->vertex_buf);
- vtx->vertex_buf = NULL;
-  }
+  _mesa_align_free(vtx->vertex_buf);
+  vtx->vertex_buf = NULL;

for (fp = vtx->fastpath ; fp ; fp = tmp) {
   tmp = fp->next;
diff --git a/src/mesa/vbo/vbo_exec_api.c b/src/mesa/vbo/vbo_exec_api.c
index 600398c..f3c41e0 100644
--- a/src/mesa/vbo/vbo_exec_api.c
+++ b/src/mesa/vbo/vbo_exec_api.c
@@ -989,11 +989,10 @@ void vbo_use_buffer_objects(struct gl_context *ctx)

 /* Make sure this func is only used once */
 assert(exec->vtx.bufferobj == ctx->Shared->NullBufferObj);
-   if (exec->vtx.buffer_map) {
-  _mesa_alig

[Mesa-dev] [PATCH] i965/gen7: Apply non-msrt fast color clear w/a to all gen7

[Chad Versace]

The BSpec states that for IVB, VLVT, and HSW, the aligment for the
non-msrt clear rectangle must be doubled.  Commit 9a1a67b applied the
workaround to Haswell GT3.  Commit 8b659ce expanded the workaround to
all Haswell variants. This commit expands it to all Gen7 hardware, as
required by the BSpec.

No Piglit regressions on Ivybridge 0x0166. No fixes either.

I know no Ivybridge nor Baytrail bug related to this workaround.
However, the BSpec says the extra alignment is required, so let's do it.

CC: "9.2, 10.0" 
CC: Anuj Phogat 
CC: Paul Berry 
Signed-off-by: Chad Versace 
---
 src/mesa/drivers/dri/i965/brw_blorp_clear.cpp | 6 +-
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/brw_blorp_clear.cpp 
b/src/mesa/drivers/dri/i965/brw_blorp_clear.cpp
index 3f096b5..e248ebf 100644
--- a/src/mesa/drivers/dri/i965/brw_blorp_clear.cpp
+++ b/src/mesa/drivers/dri/i965/brw_blorp_clear.cpp
@@ -265,7 +265,7 @@ brw_blorp_clear_params::brw_blorp_clear_params(struct 
brw_context *brw,
   x_align *= 16;
   y_align *= 32;
 
-  if (brw->is_haswell) {
+  if (brw->gen == 7) {
  /* From BSpec: 3D-Media-GPGPU Engine > 3D Pipeline > Pixel > Pixel
   * Backend > MCS Buffer for Render Target(s) [DevIVB+] > Table "Color
   * Clear of Non-MultiSampled Render Target Restrictions":
@@ -274,10 +274,6 @@ brw_blorp_clear_params::brw_blorp_clear_params(struct 
brw_context *brw,
   *   the number of pixels in the table shown below...  x_align,
   *   y_align values computed above are the relevant entries in the
   *   referred table.
-  *
-  * We apply the workaround to only Haswell because (a) we suspect that
-  * is the only hardware where it is actually required and (b) we
-  * haven't yet validated the workaround for the other hardware.
   */
  x0 = ROUND_DOWN_TO(x0, 2 * x_align);
  y0 = ROUND_DOWN_TO(y0, 2 * y_align);
-- 
1.8.4

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

Re: [Mesa-dev] [PATCH 1/2] Allow for GLSL shaders to have #extension directive anywhere in source file.

[Kenneth Graunke]

On 12/03/2013 08:43 AM, Ian Romanick wrote:
> Can we please hold off on this?  I'm in the process of getting the GLSL
> spec updated to match what people actually ship.  We've always taken the
> approach of following the spec, and if the spec doesn't match reality,
> fix the spec.  I've done this about a dozen times over the last 3 years,
> and I don't feel like there's a compelling reason to deviate for this case.
> 
> I also don't think the behavior implemented by this patch exactly
> matches other vendors.  I don't want to put a set of hacks in now only
> to replace them with something different in a few weeks.

I agree that we should hold off until the Khronos spec bug is resolved.

We'll likely want something like this (or Brian's?), but we'll know
exactly what it should look like then.

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

Re: [Mesa-dev] [PATCH V2 00/17] Newbie Project : Enable ARB_map_buffer_alignment in all drivers

[Eric Anholt]

Siavash Eliasi  writes:

> Hello, this is V2 series of patches to accomplish *Enable
> ARB_map_buffer_alignment in all drivers* newbie project suggested by
> Ian Romanick.

I think there's a piece missing to this, which is this bit of spec text:

"If no error occurs, the pointer value returned by MapBufferRange must
reflect an allocation aligned to the value of MIN_MAP_BUFFER_ALIGNMENT
basic machine units.  Subtracting  basic machine units from the
returned pointer will always produce a multiple of the value of
MIN_MAP_BUFFER_ALIGNMENT."

In i965's intel_bufferobj_map_range, range_map_bo or range_map_buffer
mappings won't have that alignment.


pgp1KgtzGANxM.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 1/9] i965: Don't print extra (null) arguments in dump_instruction().

[Eric Anholt]

Thanks!  These are:

Reviewed-by: Eric Anholt 


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

[Mesa-dev] [Bug 72281] New: [bisected] Eufloria HD crash on start

[bugzilla-daemon]

https://bugs.freedesktop.org/show_bug.cgi?id=72281

  Priority: medium
Bug ID: 72281
  Keywords: regression
CC: e...@anholt.net
  Assignee: mesa-dev@lists.freedesktop.org
   Summary: [bisected] Eufloria HD crash on start
  Severity: normal
Classification: Unclassified
OS: All
  Reporter: pavel.ondra...@email.cz
  Hardware: Other
Status: NEW
   Version: git
 Component: Mesa core
   Product: Mesa

Eufloria HD crash on start here with all tested drivers (llvmpipe, softpipe,
r300g). This is a regression introduced by:

e5885c119de1e508099cce1c9f8ff00fab88 is the first bad commit
commit e5885c119de1e508099cce1c9f8ff00fab88
Author: Eric Anholt 
Date:   Fri Sep 20 10:13:32 2013 -0700

mesa: Dynamically allocate the storage for program local parameters.

The array was 64kb per struct gl_program, plus we statically stored a copy
of one on disk for _mesa_DummyProgram.  Given that most struct gl_programs
we generate are for GLSL shaders that don't have local parameters, this
was a waste.

Since you can store and fetch parameters beyond what the program actually
uses, we do have to do a late allocation if necessary at
GetProgramLocalParameter time.

Reduces peak memory usage in the dota2 trace I made by 76MB (4.5%)

Reviewed-by: Brian Paul 
Reviewed-by: Ian Romanick 


Program received signal SIGSEGV, Segmentation fault.
__memcpy_ia32 () at ../sysdeps/i386/i686/memcpy.S:74
74rep
#0  __memcpy_ia32 () at ../sysdeps/i386/i686/memcpy.S:74
#1  0x0001 in ?? ()
#2  0xb6336a15 in _mesa_ProgramLocalParameters4fvEXT (target=34336, index=1,
count=4, params=0x87c8f48) at main/arbprogram.c:551
#3  0x4e9b3dbc in ?? () from /usr/local/games/EufloriaHD/lib/libCgGL.so
#4  0x4e9afc6a in ?? () from /usr/local/games/EufloriaHD/lib/libCgGL.so
#5  0x4e9af055 in ?? () from /usr/local/games/EufloriaHD/lib/libCgGL.so
#6  0x4e987702 in ?? () from /usr/local/games/EufloriaHD/lib/libCgGL.so
#7  0x4e987819 in ?? () from /usr/local/games/EufloriaHD/lib/libCgGL.so
#8  0x4e9899ee in cgGLLoadProgram () from
/usr/local/games/EufloriaHD/lib/libCgGL.so
#9  0x081e85f8 in Coal::Shader::LoadBinary(char const*) ()
#10 0x081e7a06 in Coal::Win32Renderer::LoadShaderBinary(char const*, bool) ()
#11 0x080e64dd in Drawing::Drawing() ()
#12 0x080fb7c4 in EufloriaApp::Initialise() ()
#13 0x081db653 in Coal::App::Run() ()
#14 0x081579a5 in main ()

-- 
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 1/3] glx: Check malloc return value before accessing memory in glx/clientattrib.c

[Juha-Pekka Heikkilä]

On Tue, Dec 3, 2013 at 6:39 PM, Brian Paul  wrote:
> On 12/02/2013 02:39 AM, Juha-Pekka Heikkila wrote:
>>
>> Signed-off-by: Juha-Pekka Heikkila 
>> ---
>>   src/glx/clientattrib.c | 5 +
>>   1 file changed, 5 insertions(+)
>>
>> diff --git a/src/glx/clientattrib.c b/src/glx/clientattrib.c
>> index 1b306ea..a26906f 100644
>> --- a/src/glx/clientattrib.c
>> +++ b/src/glx/clientattrib.c
>> @@ -76,6 +76,11 @@ __indirect_glPushClientAttrib(GLuint mask)
>>  if (spp < &gc->attributes.stack[__GL_CLIENT_ATTRIB_STACK_DEPTH]) {
>> if (!(sp = *spp)) {
>>sp = malloc(sizeof(__GLXattribute));
>> +
>> + if (sp == NULL) {
>> +__glXSetError(gc, GL_OUT_OF_MEMORY);
>> +return;
>> + }
>>*spp = sp;
>> }
>> sp->mask = mask;
>>
>
> Reviewed-by: Brian Paul 
>
> Need someone to push this for you?
>

Thank you, it would be good if someone can push the patches that look
good since I cannot push these.

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

[Mesa-dev] [PATCH 1/4] radeon: rearrange r600_texture and related code a bit.

[Andreas Hartmetz]

This should make the differences and similarities between color and
depth buffer handling more clear.
---
 src/gallium/drivers/r600/evergreen_state.c| 10 ++---
 src/gallium/drivers/r600/r600_blit.c  |  6 +--
 src/gallium/drivers/r600/r600_state.c | 10 ++---
 src/gallium/drivers/radeon/r600_pipe_common.h |  8 ++--
 src/gallium/drivers/radeon/r600_texture.c | 58 +--
 5 files changed, 46 insertions(+), 46 deletions(-)

diff --git a/src/gallium/drivers/r600/evergreen_state.c 
b/src/gallium/drivers/r600/evergreen_state.c
index a4a4e3e..0342574 100644
--- a/src/gallium/drivers/r600/evergreen_state.c
+++ b/src/gallium/drivers/r600/evergreen_state.c
@@ -1696,8 +1696,8 @@ static void evergreen_init_depth_surface(struct 
r600_context *rctx,
 
surf->htile_enabled = 0;
/* use htile only for first level */
-   if (rtex->htile && !level) {
-   uint64_t va = r600_resource_va(&rctx->screen->b.b, 
&rtex->htile->b.b);
+   if (rtex->htile_buffer && !level) {
+   uint64_t va = r600_resource_va(&rctx->screen->b.b, 
&rtex->htile_buffer->b.b);
surf->htile_enabled = 1;
surf->db_htile_data_base = va >> 8;
surf->db_htile_surface = S_028ABC_HTILE_WIDTH(1) |
@@ -1732,7 +1732,7 @@ static void evergreen_set_framebuffer_state(struct 
pipe_context *ctx,
rctx->b.flags |= R600_CONTEXT_FLUSH_AND_INV_DB;
 
rtex = (struct 
r600_texture*)rctx->framebuffer.state.zsbuf->texture;
-   if (rtex->htile) {
+   if (rtex->htile_buffer) {
rctx->b.flags |= R600_CONTEXT_FLUSH_AND_INV_DB_META;
}
}
@@ -2362,11 +2362,11 @@ static void evergreen_emit_db_state(struct r600_context 
*rctx, struct r600_atom
struct r600_texture *rtex = (struct r600_texture 
*)a->rsurf->base.texture;
unsigned reloc_idx;
 
-   r600_write_context_reg(cs, R_02802C_DB_DEPTH_CLEAR, 
fui(rtex->depth_clear));
+   r600_write_context_reg(cs, R_02802C_DB_DEPTH_CLEAR, 
fui(rtex->depth_clear_value));
r600_write_context_reg(cs, R_028ABC_DB_HTILE_SURFACE, 
a->rsurf->db_htile_surface);
r600_write_context_reg(cs, R_028AC8_DB_PRELOAD_CONTROL, 
a->rsurf->db_preload_control);
r600_write_context_reg(cs, R_028014_DB_HTILE_DATA_BASE, 
a->rsurf->db_htile_data_base);
-   reloc_idx = r600_context_bo_reloc(&rctx->b, &rctx->b.rings.gfx, 
rtex->htile, RADEON_USAGE_READWRITE);
+   reloc_idx = r600_context_bo_reloc(&rctx->b, &rctx->b.rings.gfx, 
rtex->htile_buffer, RADEON_USAGE_READWRITE);
cs->buf[cs->cdw++] = PKT3(PKT3_NOP, 0, 0);
cs->buf[cs->cdw++] = reloc_idx;
} else {
diff --git a/src/gallium/drivers/r600/r600_blit.c 
b/src/gallium/drivers/r600/r600_blit.c
index f33bb43..42034a0 100644
--- a/src/gallium/drivers/r600/r600_blit.c
+++ b/src/gallium/drivers/r600/r600_blit.c
@@ -547,9 +547,9 @@ static void r600_clear(struct pipe_context *ctx, unsigned 
buffers,
 * disable fast clear for texture array.
 */
/* Only use htile for first level */
-   if (rtex->htile && !level && rtex->surface.array_size == 1) {
-   if (rtex->depth_clear != depth) {
-   rtex->depth_clear = depth;
+   if (rtex->htile_buffer && !level && rtex->surface.array_size == 
1) {
+   if (rtex->depth_clear_value != depth) {
+   rtex->depth_clear_value = depth;
rctx->db_state.atom.dirty = true;
}
rctx->db_misc_state.htile_clear = true;
diff --git a/src/gallium/drivers/r600/r600_state.c 
b/src/gallium/drivers/r600/r600_state.c
index 41e9c5d..2cff22c 100644
--- a/src/gallium/drivers/r600/r600_state.c
+++ b/src/gallium/drivers/r600/r600_state.c
@@ -1533,8 +1533,8 @@ static void r600_init_depth_surface(struct r600_context 
*rctx,
 
surf->htile_enabled = 0;
/* use htile only for first level */
-   if (rtex->htile && !level) {
-   uint64_t va = r600_resource_va(&rctx->screen->b.b, 
&rtex->htile->b.b);
+   if (rtex->htile_buffer && !level) {
+   uint64_t va = r600_resource_va(&rctx->screen->b.b, 
&rtex->htile_buffer->b.b);
surf->htile_enabled = 1;
surf->db_htile_data_base = va >> 8;
surf->db_htile_surface = S_028D24_HTILE_WIDTH(1) |
@@ -1570,7 +1570,7 @@ static void r600_set_framebuffer_state(struct 
pipe_context *ctx,
rctx->b.flags |= R600_CONTEXT_FLUSH_AND_INV_DB;
 
rtex = (struct 
r600_texture*)rctx->framebuffer.state.zsbuf->texture;
-   if (rctx->b.chip_class >= R700 && rtex->htile) {
+   if (rctx->b.chip_class >= R700 && rtex->htile_buffer) {
  

[Mesa-dev] [PATCH 3/4] radeonsi: Rearrange si_create_dsa_state

[Andreas Hartmetz]

Reduce scope of variables and divide the code more clearly into
sections dealing with one thing.
---
 src/gallium/drivers/radeonsi/si_state.c | 38 +++--
 1 file changed, 22 insertions(+), 16 deletions(-)

diff --git a/src/gallium/drivers/radeonsi/si_state.c 
b/src/gallium/drivers/radeonsi/si_state.c
index 7bae72a..24c9cf3 100644
--- a/src/gallium/drivers/radeonsi/si_state.c
+++ b/src/gallium/drivers/radeonsi/si_state.c
@@ -706,25 +706,20 @@ static void *si_create_dsa_state(struct pipe_context *ctx,
 const struct pipe_depth_stencil_alpha_state 
*state)
 {
struct si_state_dsa *dsa = CALLOC_STRUCT(si_state_dsa);
-   struct si_pm4_state *pm4 = &dsa->pm4;
-   unsigned db_depth_control;
-   unsigned db_render_override, db_render_control;
-   uint32_t db_stencil_control = 0;
-
if (dsa == NULL) {
return NULL;
}
 
+   unsigned db_depth_control = 0;
+
+   /* stencil */
+
dsa->valuemask[0] = state->stencil[0].valuemask;
dsa->valuemask[1] = state->stencil[1].valuemask;
dsa->writemask[0] = state->stencil[0].writemask;
dsa->writemask[1] = state->stencil[1].writemask;
 
-   db_depth_control = S_028800_Z_ENABLE(state->depth.enabled) |
-   S_028800_Z_WRITE_ENABLE(state->depth.writemask) |
-   S_028800_ZFUNC(state->depth.func);
-
-   /* stencil */
+   uint32_t db_stencil_control = 0;
if (state->stencil[0].enabled) {
db_depth_control |= S_028800_STENCIL_ENABLE(1);
db_depth_control |= 
S_028800_STENCILFUNC(state->stencil[0].func);
@@ -740,8 +735,13 @@ static void *si_create_dsa_state(struct pipe_context *ctx,
db_stencil_control |= 
S_02842C_STENCILZFAIL_BF(si_translate_stencil_op(state->stencil[1].zfail_op));
}
}
+   unsigned db_render_override = 
S_02800C_FORCE_HIS_ENABLE0(V_02800C_FORCE_DISABLE) |
+ 
S_02800C_FORCE_HIS_ENABLE1(V_02800C_FORCE_DISABLE);
 
/* alpha */
+
+   struct si_pm4_state *pm4 = &dsa->pm4;
+
if (state->alpha.enabled) {
dsa->alpha_func = state->alpha.func;
dsa->alpha_ref = state->alpha.ref_value;
@@ -752,12 +752,19 @@ static void *si_create_dsa_state(struct pipe_context *ctx,
dsa->alpha_func = PIPE_FUNC_ALWAYS;
}
 
-   /* misc */
-   db_render_control = 0;
-   db_render_override = S_02800C_FORCE_HIZ_ENABLE(V_02800C_FORCE_DISABLE) |
-   S_02800C_FORCE_HIS_ENABLE0(V_02800C_FORCE_DISABLE) |
-   S_02800C_FORCE_HIS_ENABLE1(V_02800C_FORCE_DISABLE);
+   /* depth */
+
+   db_depth_control |= S_028800_Z_ENABLE(state->depth.enabled) |
+   S_028800_Z_WRITE_ENABLE(state->depth.writemask) |
+   S_028800_ZFUNC(state->depth.func);
+   db_render_override |= S_02800C_FORCE_HIZ_ENABLE(V_02800C_FORCE_DISABLE);
+
+   /* write remaining registers and return */
+
/* TODO db_render_override depends on query */
+   dsa->db_render_override = db_render_override;
+   unsigned db_render_control = 0;
+
si_pm4_set_reg(pm4, R_028020_DB_DEPTH_BOUNDS_MIN, 0x);
si_pm4_set_reg(pm4, R_028024_DB_DEPTH_BOUNDS_MAX, 0x);
si_pm4_set_reg(pm4, R_028028_DB_STENCIL_CLEAR, 0x);
@@ -770,7 +777,6 @@ static void *si_create_dsa_state(struct pipe_context *ctx,
si_pm4_set_reg(pm4, R_028AC0_DB_SRESULTS_COMPARE_STATE0, 0x0);
si_pm4_set_reg(pm4, R_028AC4_DB_SRESULTS_COMPARE_STATE1, 0x0);
si_pm4_set_reg(pm4, R_028AC8_DB_PRELOAD_CONTROL, 0x0);
-   dsa->db_render_override = db_render_override;
 
return dsa;
 }
-- 
1.8.3.2

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

[Mesa-dev] [PATCH 4/4] radeonsi: Write htile state to SI hardware.

[Andreas Hartmetz]

---
 src/gallium/drivers/radeonsi/si_state.c | 28 +++-
 1 file changed, 27 insertions(+), 1 deletion(-)

diff --git a/src/gallium/drivers/radeonsi/si_state.c 
b/src/gallium/drivers/radeonsi/si_state.c
index 24c9cf3..7b1a6df 100644
--- a/src/gallium/drivers/radeonsi/si_state.c
+++ b/src/gallium/drivers/radeonsi/si_state.c
@@ -757,7 +757,19 @@ static void *si_create_dsa_state(struct pipe_context *ctx,
db_depth_control |= S_028800_Z_ENABLE(state->depth.enabled) |
S_028800_Z_WRITE_ENABLE(state->depth.writemask) |
S_028800_ZFUNC(state->depth.func);
-   db_render_override |= S_02800C_FORCE_HIZ_ENABLE(V_02800C_FORCE_DISABLE);
+
+   struct r600_context *rctx = (struct r600_context *)ctx;
+   struct pipe_surface *surf = rctx->framebuffer.zsbuf;
+   struct r600_texture *rtex = 0;
+   if (surf) {
+   rtex = (struct r600_texture*)surf->texture;
+   }
+   if (rtex && rtex->htile_buffer) {
+   /* Force off means no force, DB_SHADER_CONTROL decides */
+   db_render_override |= 
S_02800C_FORCE_HIZ_ENABLE(V_02800C_FORCE_OFF);
+   } else {
+   db_render_override |= 
S_02800C_FORCE_HIZ_ENABLE(V_02800C_FORCE_DISABLE);
+   }
 
/* write remaining registers and return */
 
@@ -1842,9 +1854,21 @@ static void si_db(struct r600_context *rctx, struct 
si_pm4_state *pm4,
s_info |= S_028044_TILE_MODE_INDEX(tile_mode_index);
}
 
+   uint32_t db_htile_data_base = 0;
+   uint32_t db_htile_surface = 0;
+   /* use htile only for first level */
+   if (rtex->htile_buffer && !level) {
+   z_info |= S_028040_TILE_SURFACE_ENABLE(1);
+   uint64_t va = r600_resource_va(&rctx->screen->b.b, 
&rtex->htile_buffer->b.b);
+   db_htile_data_base = va >> 8;
+
+   db_htile_surface = S_028ABC_FULL_CACHE(1);
+   }
+
si_pm4_set_reg(pm4, R_028008_DB_DEPTH_VIEW,
   S_028008_SLICE_START(state->zsbuf->u.tex.first_layer) |
   S_028008_SLICE_MAX(state->zsbuf->u.tex.last_layer));
+   si_pm4_set_reg(pm4, R_028014_DB_HTILE_DATA_BASE, db_htile_data_base);
 
si_pm4_set_reg(pm4, R_02803C_DB_DEPTH_INFO, db_depth_info);
si_pm4_set_reg(pm4, R_028040_DB_Z_INFO, z_info);
@@ -1858,6 +1882,8 @@ static void si_db(struct r600_context *rctx, struct 
si_pm4_state *pm4,
 
si_pm4_set_reg(pm4, R_028058_DB_DEPTH_SIZE, 
S_028058_PITCH_TILE_MAX(pitch));
si_pm4_set_reg(pm4, R_02805C_DB_DEPTH_SLICE, 
S_02805C_SLICE_TILE_MAX(slice));
+
+   si_pm4_set_reg(pm4, R_028ABC_DB_HTILE_SURFACE, db_htile_surface);
 }
 
 #define FILL_SREG(s0x, s0y, s1x, s1y, s2x, s2y, s3x, s3y)  \
-- 
1.8.3.2

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

[Mesa-dev] radeonsi htile (aka HyperZ) support

[Andreas Hartmetz]

This adds HyperZ support, without fast clear for now.
I got a lot of help from Marek with this.
There seem to be no piglit regressions; about 60 very basic GL1.0 tests
were skipped in my test run for some reason - they looked very
unrelated. There was no change in other tests.

It does seem to make a difference for performance - running a Xonotic
demo with custom high graphics settings on a Radeon HD 7750 -
Before:
2351 frames 117.2196701 seconds 20.0563608 fps,
 one-second fps min/avg/max: 14 21 28 (75 seconds)
After:
2590 frames 116.1412549 seconds 22.3004306 fps,
 one-second fps min/avg/max: 16 23 30 (82 seconds)

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

[Mesa-dev] [PATCH 2/4] radeon: Allocate htile buffer for SI in r600_texture.

[Andreas Hartmetz]

---
 src/gallium/drivers/radeon/r600_texture.c | 84 +--
 1 file changed, 68 insertions(+), 16 deletions(-)

diff --git a/src/gallium/drivers/radeon/r600_texture.c 
b/src/gallium/drivers/radeon/r600_texture.c
index 70f21bd..c91fbce 100644
--- a/src/gallium/drivers/radeon/r600_texture.c
+++ b/src/gallium/drivers/radeon/r600_texture.c
@@ -457,12 +457,53 @@ void r600_texture_init_cmask(struct r600_common_screen 
*rscreen,
}
 }
 
-static void r600_texture_allocate_htile(struct r600_common_screen *rscreen,
-   struct r600_texture *rtex)
+static unsigned si_texture_htile_alloc_size(struct r600_common_screen *rscreen,
+   struct r600_texture *rtex)
+{
+   unsigned num_pipes = rscreen->tiling_info.num_channels;
+
+   unsigned cl_width;
+   unsigned cl_height;
+   switch (num_pipes) {
+   case 2:
+   cl_width = 32;
+   cl_height = 32;
+   break;
+   case 4:
+   cl_width = 64;
+   cl_height = 32;
+   break;
+   case 8:
+   cl_width = 64;
+   cl_height = 64;
+   break;
+   case 16:
+   cl_width = 128;
+   cl_height = 64;
+   break;
+   default:
+   assert(0);
+   return 0;
+   }
+
+   unsigned width = align(rtex->surface.npix_x, cl_width * 8);
+   unsigned height = align(rtex->surface.npix_y, cl_height * 8);
+
+   unsigned slice_elements = (width * height) / (8 * 8);
+   unsigned slice_bytes = slice_elements * 4;
+
+   unsigned pipe_interleave_bytes = rscreen->tiling_info.group_bytes;
+   unsigned base_align = num_pipes * pipe_interleave_bytes;
+   unsigned size = rtex->surface.array_size * align(slice_bytes, 
base_align);
+
+   return size;
+}
+
+static unsigned r600_texture_htile_alloc_size(struct r600_common_screen 
*rscreen,
+ struct r600_texture *rtex)
 {
unsigned sw = rtex->surface.level[0].nblk_x * rtex->surface.blk_w;
unsigned sh = rtex->surface.level[0].nblk_y * rtex->surface.blk_h;
-   unsigned htile_size;
unsigned npipes = rscreen->info.r600_num_tile_pipes;
 
/* XXX also use it for other texture targets */
@@ -470,24 +511,39 @@ static void r600_texture_allocate_htile(struct 
r600_common_screen *rscreen,
rtex->resource.b.b.target != PIPE_TEXTURE_2D ||
rtex->surface.level[0].nblk_x < 32 ||
rtex->surface.level[0].nblk_y < 32) {
-   return;
+   return 0;
}
 
/* this alignment and htile size only apply to linear htile buffer */
sw = align(sw, 16 << 3);
sh = align(sh, npipes << 3);
-   htile_size = (sw >> 3) * (sh >> 3) * 4;
+   unsigned htile_size = (sw >> 3) * (sh >> 3) * 4;
/* must be aligned with 2K * npipes */
htile_size = align(htile_size, (2 << 10) * npipes);
+   return htile_size;
+}
+
+static void r600_texture_allocate_htile(struct r600_common_screen *rscreen,
+   struct r600_texture *rtex)
+{
+   unsigned htile_size;
+   if (rscreen->chip_class >= SI) {
+   htile_size = si_texture_htile_alloc_size(rscreen, rtex);
+   } else {
+   htile_size = r600_texture_htile_alloc_size(rscreen, rtex);
+   }
+
+   if (!htile_size)
+   return;
 
/* XXX don't allocate it separately */
-   rtex->htile_buffer = (struct 
r600_resource*)pipe_buffer_create(&rscreen->b, PIPE_BIND_CUSTOM,
-  
PIPE_USAGE_STATIC, htile_size);
+   rtex->htile_buffer = (struct r600_resource*)
+pipe_buffer_create(&rscreen->b, PIPE_BIND_CUSTOM,
+   PIPE_USAGE_STATIC, htile_size);
if (rtex->htile_buffer == NULL) {
/* this is not a fatal error as we can still keep rendering
-* without htile buffer
-*/
-   R600_ERR("r600: failed to create bo for htile buffers\n");
+* without htile buffer */
+   R600_ERR("Failed to create buffer object for htile buffer.\n");
} else {
r600_screen_clear_buffer(rscreen, &rtex->htile_buffer->b.b, 0, 
htile_size, 0);
}
@@ -534,12 +590,8 @@ r600_texture_create_object(struct pipe_screen *screen,
if (!(base->flags & (R600_RESOURCE_FLAG_TRANSFER |
 R600_RESOURCE_FLAG_FLUSHED_DEPTH)) &&
!(rscreen->debug_flags & DBG_NO_HYPERZ)) {
-   if (rscreen->chip_class >= SI) {
-   /* XXX implement Hyper-Z for SI.
-* Reuse the CMASK allocator, which is almost 
the same as HTILE. */
-

Re: [Mesa-dev] [PATCH] glxinfo: print ES2 extension list

[Ian Romanick]

I like the idea.  I haven't really looked at the code, so...

Acked-by: Ian Romanick 

But there is one comment below...

On 12/03/2013 09:58 AM, Marek Olšák wrote:
> From: Marek Olšák 
> 
> ---
>  src/xdemos/glxinfo.c | 43 ++-
>  1 file changed, 30 insertions(+), 13 deletions(-)
> 
> diff --git a/src/xdemos/glxinfo.c b/src/xdemos/glxinfo.c
> index c9edc6a..a008ea8 100644
> --- a/src/xdemos/glxinfo.c
> +++ b/src/xdemos/glxinfo.c
> @@ -760,7 +760,7 @@ create_context_flags(Display *dpy, GLXFBConfig fbconfig, 
> int major, int minor,
>   */
>  static GLXContext
>  create_context_with_config(Display *dpy, GLXFBConfig config,
> -   Bool coreProfile, Bool direct)
> +   Bool coreProfile, Bool es2Profile, Bool direct)
>  {
> GLXContext ctx = 0;
>  
> @@ -787,6 +787,21 @@ create_context_with_config(Display *dpy, GLXFBConfig 
> config,
>return 0;
> }
>  
> +   if (es2Profile) {
> +#ifdef GLX_CONTEXT_ES2_PROFILE_BIT_EXT
> +  if (extension_supported("GLX_EXT_create_context_es2_profile",
> +  glXQueryExtensionsString(dpy, 0))) {
> + ctx = create_context_flags(dpy, config, 2, 0, 0x0,
> +GLX_CONTEXT_ES2_PROFILE_BIT_EXT,
> +direct);
> + return ctx;
> +  }
> +  return 0;
> +#else
> +  return 0;
> +#endif

This looks weird.  Maybe just one "return 0" outside the #ifdef block?

> +   }
> +
> /* GLX should return a context of the latest GL version that supports
>  * the full profile.
>  */
> @@ -832,8 +847,8 @@ choose_xvisinfo(Display *dpy, int scrnum)
>  
>  static Bool
>  print_screen_info(Display *dpy, int scrnum, Bool allowDirect,
> -  Bool coreProfile, Bool limits, Bool singleLine,
> -  Bool coreWorked)
> +  Bool coreProfile, Bool es2Profile, Bool limits,
> +   Bool singleLine, Bool coreWorked)
>  {
> Window win;
> XSetWindowAttributes attr;
> @@ -843,7 +858,8 @@ print_screen_info(Display *dpy, int scrnum, Bool 
> allowDirect,
> XVisualInfo *visinfo;
> int width = 100, height = 100;
> GLXFBConfig *fbconfigs;
> -   const char *oglstring = coreProfile ? "OpenGL core profile" : "OpenGL";
> +   const char *oglstring = coreProfile ? "OpenGL core profile" :
> +es2Profile ? "OpenGL ES profile" : "OpenGL";
>  
> root = RootWindow(dpy, scrnum);
>  
> @@ -853,30 +869,30 @@ print_screen_info(Display *dpy, int scrnum, Bool 
> allowDirect,
> fbconfigs = choose_fb_config(dpy, scrnum);
> if (fbconfigs) {
>ctx = create_context_with_config(dpy, fbconfigs[0],
> -   coreProfile, allowDirect);
> +   coreProfile, es2Profile, allowDirect);
>if (!ctx && allowDirect && !coreProfile) {
>   /* try indirect */
>   ctx = create_context_with_config(dpy, fbconfigs[0],
> -  coreProfile, False);
> +  coreProfile, es2Profile, False);
>}
>  
>visinfo = glXGetVisualFromFBConfig(dpy, fbconfigs[0]);
>XFree(fbconfigs);
> }
> -   else if (!coreProfile) {
> +   else if (!coreProfile && !es2Profile) {
>visinfo = choose_xvisinfo(dpy, scrnum);
>if (visinfo)
>ctx = glXCreateContext(dpy, visinfo, NULL, allowDirect);
> } else
>visinfo = NULL;
>  
> -   if (!visinfo && !coreProfile) {
> +   if (!visinfo && !coreProfile && !es2Profile) {
>fprintf(stderr, "Error: couldn't find RGB GLX visual or fbconfig\n");
>return False;
> }
>  
> if (!ctx) {
> -  if (!coreProfile)
> +  if (!coreProfile && !es2Profile)
>fprintf(stderr, "Error: glXCreateContext failed\n");
>XFree(visinfo);
>return False;
> @@ -990,7 +1006,7 @@ print_screen_info(Display *dpy, int scrnum, Bool 
> allowDirect,
>  #endif
>CheckError(__LINE__);
>  #ifdef GL_VERSION_3_0
> -  if (version >= 30) {
> +  if (version >= 30 && !es2Profile) {
>   GLint flags;
>   glGetIntegerv(GL_CONTEXT_FLAGS, &flags);
>   printf("%s context flags: %s\n", oglstring, 
> context_flags_string(flags));
> @@ -998,7 +1014,7 @@ print_screen_info(Display *dpy, int scrnum, Bool 
> allowDirect,
>  #endif
>CheckError(__LINE__);
>  #ifdef GL_VERSION_3_2
> -  if (version >= 32) {
> +  if (version >= 32 && !es2Profile) {
>   GLint mask;
>   glGetIntegerv(GL_CONTEXT_PROFILE_MASK, &mask);
>   printf("%s profile mask: %s\n", oglstring, 
> profile_mask_string(mask));
> @@ -1730,8 +1746,9 @@ main(int argc, char *argv[])
>print_display_info(dpy);
>for (scrnum = 0; scrnum < numScreens; scrnum++) {
>   mesa_hack(dpy, scrnum);
> - coreWorked = print_screen_info(dpy, scrnum, allowDirect, 

Re: [Mesa-dev] [PATCH 3/3] mesa: Verify memory allocations success in _mesa_PushAttrib

[Juha-Pekka Heikkilä]

On Tue, Dec 3, 2013 at 6:51 PM, Ian Romanick  wrote:
> On 12/02/2013 01:39 AM, Juha-Pekka Heikkila wrote:
>> Check if any of the callocs fail and report it with _mesa_error
>> if needed.
>>
>> Signed-off-by: Juha-Pekka Heikkila 
>> ---
>>  src/mesa/main/attrib.c | 106 
>> -
>>  1 file changed, 104 insertions(+), 2 deletions(-)
>>
>> diff --git a/src/mesa/main/attrib.c b/src/mesa/main/attrib.c
>> index 2418fb0..037cd64 100644
>> --- a/src/mesa/main/attrib.c
>> +++ b/src/mesa/main/attrib.c
>> @@ -222,6 +222,11 @@ _mesa_PushAttrib(GLbitfield mask)
>> if (mask & GL_ACCUM_BUFFER_BIT) {
>>struct gl_accum_attrib *attr;
>>attr = MALLOC_STRUCT( gl_accum_attrib );
>> +  if (attr == NULL) {
>> + _mesa_error(ctx, GL_OUT_OF_MEMORY, "glPushAttrib");
>> + goto end;
>> +  }
>> +
>>memcpy( attr, &ctx->Accum, sizeof(struct gl_accum_attrib) );
>>save_attrib_data(&head, GL_ACCUM_BUFFER_BIT, attr);
>
> And what if save_attrib_data fails to allocate memory for head?

I see I read save_attrib_data function too quick. Looks this would
leak the memory allocated in attr if save_attrib_data fails, I'll add
few bits here. Thanks Ian.

>
>> }
>> @@ -230,6 +235,11 @@ _mesa_PushAttrib(GLbitfield mask)
>>GLuint i;
>>struct gl_colorbuffer_attrib *attr;
>>attr = MALLOC_STRUCT( gl_colorbuffer_attrib );
>> +  if (attr == NULL) {
>> + _mesa_error(ctx, GL_OUT_OF_MEMORY, "glPushAttrib");
>> + goto end;
>> +  }
>> +
>>memcpy( attr, &ctx->Color, sizeof(struct gl_colorbuffer_attrib) );
>>/* push the Draw FBO's DrawBuffer[] state, not 
>> ctx->Color.DrawBuffer[] */
>>for (i = 0; i < ctx->Const.MaxDrawBuffers; i ++)
>> @@ -241,6 +251,11 @@ _mesa_PushAttrib(GLbitfield mask)
>>struct gl_current_attrib *attr;
>>FLUSH_CURRENT( ctx, 0 );
>>attr = MALLOC_STRUCT( gl_current_attrib );
>> +  if (attr == NULL) {
>> + _mesa_error(ctx, GL_OUT_OF_MEMORY, "glPushAttrib");
>> + goto end;
>> +  }
>> +
>>memcpy( attr, &ctx->Current, sizeof(struct gl_current_attrib) );
>>save_attrib_data(&head, GL_CURRENT_BIT, attr);
>> }
>> @@ -248,6 +263,11 @@ _mesa_PushAttrib(GLbitfield mask)
>> if (mask & GL_DEPTH_BUFFER_BIT) {
>>struct gl_depthbuffer_attrib *attr;
>>attr = MALLOC_STRUCT( gl_depthbuffer_attrib );
>> +  if (attr == NULL) {
>> + _mesa_error(ctx, GL_OUT_OF_MEMORY, "glPushAttrib");
>> + goto end;
>> +  }
>> +
>>memcpy( attr, &ctx->Depth, sizeof(struct gl_depthbuffer_attrib) );
>>save_attrib_data(&head, GL_DEPTH_BUFFER_BIT, attr);
>> }
>> @@ -256,6 +276,11 @@ _mesa_PushAttrib(GLbitfield mask)
>>struct gl_enable_attrib *attr;
>>GLuint i;
>>attr = MALLOC_STRUCT( gl_enable_attrib );
>> +  if (attr == NULL) {
>> + _mesa_error(ctx, GL_OUT_OF_MEMORY, "glPushAttrib");
>> + goto end;
>> +  }
>> +
>>/* Copy enable flags from all other attributes into the enable 
>> struct. */
>>attr->AlphaTest = ctx->Color.AlphaEnabled;
>>attr->AutoNormal = ctx->Eval.AutoNormal;
>> @@ -331,6 +356,11 @@ _mesa_PushAttrib(GLbitfield mask)
>> if (mask & GL_EVAL_BIT) {
>>struct gl_eval_attrib *attr;
>>attr = MALLOC_STRUCT( gl_eval_attrib );
>> +  if (attr == NULL) {
>> + _mesa_error(ctx, GL_OUT_OF_MEMORY, "glPushAttrib");
>> + goto end;
>> +  }
>> +
>>memcpy( attr, &ctx->Eval, sizeof(struct gl_eval_attrib) );
>>save_attrib_data(&head, GL_EVAL_BIT, attr);
>> }
>> @@ -338,6 +368,11 @@ _mesa_PushAttrib(GLbitfield mask)
>> if (mask & GL_FOG_BIT) {
>>struct gl_fog_attrib *attr;
>>attr = MALLOC_STRUCT( gl_fog_attrib );
>> +  if (attr == NULL) {
>> + _mesa_error(ctx, GL_OUT_OF_MEMORY, "glPushAttrib");
>> + goto end;
>> +  }
>> +
>>memcpy( attr, &ctx->Fog, sizeof(struct gl_fog_attrib) );
>>save_attrib_data(&head, GL_FOG_BIT, attr);
>> }
>> @@ -345,6 +380,11 @@ _mesa_PushAttrib(GLbitfield mask)
>> if (mask & GL_HINT_BIT) {
>>struct gl_hint_attrib *attr;
>>attr = MALLOC_STRUCT( gl_hint_attrib );
>> +  if (attr == NULL) {
>> + _mesa_error(ctx, GL_OUT_OF_MEMORY, "glPushAttrib");
>> + goto end;
>> +  }
>> +
>>memcpy( attr, &ctx->Hint, sizeof(struct gl_hint_attrib) );
>>save_attrib_data(&head, GL_HINT_BIT, attr);
>> }
>> @@ -353,6 +393,11 @@ _mesa_PushAttrib(GLbitfield mask)
>>struct gl_light_attrib *attr;
>>FLUSH_CURRENT(ctx, 0); /* flush material changes */
>>attr = MALLOC_STRUCT( gl_light_attrib );
>> +  if (attr == NULL) {
>> + _mesa_error(ctx, GL_OUT_OF_MEMORY, "glPushAttrib");
>> + goto end;
>> +  }
>> +
>>memcpy( attr, &ctx->Light, sizeof(struct gl_light_attrib) );
>>save_a

[Mesa-dev] [PATCH] i965: Add extra-alignment for non-msrt fast color clear for all hw (v2)

[Chad Versace]

The BSpec states that the aligment for the non-msrt clear rectangle must
be doubled; the BSpec does not restricit the workaround to specific
hardware.

Commit 9a1a67b applied the workaround to Haswell GT3.  Commit 8b659ce
expanded the workaround to all Haswell variants. This commit expands it
to all hardware.

No Piglit regressions on Ivybridge 0x0166. No fixes either.

I know no Ivybridge nor Baytrail bug related to this workaround.
However, the BSpec says the extra alignment is required, so let's do it.

v2: Apply to all hardware, not just gen7.

CC: "9.2, 10.0" 
CC: Anuj Phogat 
CC: Paul Berry 
Signed-off-by: Chad Versace 
---
 src/mesa/drivers/dri/i965/brw_blorp_clear.cpp | 35 +--
 1 file changed, 11 insertions(+), 24 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/brw_blorp_clear.cpp 
b/src/mesa/drivers/dri/i965/brw_blorp_clear.cpp
index 3f096b5..1904bde 100644
--- a/src/mesa/drivers/dri/i965/brw_blorp_clear.cpp
+++ b/src/mesa/drivers/dri/i965/brw_blorp_clear.cpp
@@ -265,30 +265,17 @@ brw_blorp_clear_params::brw_blorp_clear_params(struct 
brw_context *brw,
   x_align *= 16;
   y_align *= 32;
 
-  if (brw->is_haswell) {
- /* From BSpec: 3D-Media-GPGPU Engine > 3D Pipeline > Pixel > Pixel
-  * Backend > MCS Buffer for Render Target(s) [DevIVB+] > Table "Color
-  * Clear of Non-MultiSampled Render Target Restrictions":
-  *
-  *   [IVB, VLVT, HSW]: Clear rectangle must be aligned to two times
-  *   the number of pixels in the table shown below...  x_align,
-  *   y_align values computed above are the relevant entries in the
-  *   referred table.
-  *
-  * We apply the workaround to only Haswell because (a) we suspect that
-  * is the only hardware where it is actually required and (b) we
-  * haven't yet validated the workaround for the other hardware.
-  */
- x0 = ROUND_DOWN_TO(x0, 2 * x_align);
- y0 = ROUND_DOWN_TO(y0, 2 * y_align);
- x1 = ALIGN(x1, 2 * x_align);
- y1 = ALIGN(y1, 2 * y_align);
-  } else {
- x0 = ROUND_DOWN_TO(x0,  x_align);
- y0 = ROUND_DOWN_TO(y0, y_align);
- x1 = ALIGN(x1, x_align);
- y1 = ALIGN(y1, y_align);
-  }
+  /* From BSpec: 3D-Media-GPGPU Engine > 3D Pipeline > Pixel > Pixel
+   * Backend > MCS Buffer for Render Target(s) [DevIVB+] > Table "Color
+   * Clear of Non-MultiSampled Render Target Restrictions":
+   *
+   *   Clear rectangle must be aligned to two times the number of pixels in
+   *   the table shown below due to 16x16 hashing across the slice.
+   */
+  x0 = ROUND_DOWN_TO(x0, 2 * x_align);
+  y0 = ROUND_DOWN_TO(y0, 2 * y_align);
+  x1 = ALIGN(x1, 2 * x_align);
+  y1 = ALIGN(y1, 2 * y_align);
 
   /* From the Ivy Bridge PRM, Vol2 Part1 11.7 "MCS Buffer for Render
* Target(s)", beneath the "Fast Color Clear" bullet (p327):
-- 
1.8.4

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

Re: [Mesa-dev] [PATCH] i965: Add extra-alignment for non-msrt fast color clear for all hw (v2)

[Paul Berry]

On 3 December 2013 13:12, Chad Versace  wrote:

> The BSpec states that the aligment for the non-msrt clear rectangle must
> be doubled; the BSpec does not restricit the workaround to specific
> hardware.
>
> Commit 9a1a67b applied the workaround to Haswell GT3.  Commit 8b659ce
> expanded the workaround to all Haswell variants. This commit expands it
> to all hardware.
>
> No Piglit regressions on Ivybridge 0x0166. No fixes either.
>
> I know no Ivybridge nor Baytrail bug related to this workaround.
> However, the BSpec says the extra alignment is required, so let's do it.
>
> v2: Apply to all hardware, not just gen7.
>
> CC: "9.2, 10.0" 
> CC: Anuj Phogat 
> CC: Paul Berry 
> Signed-off-by: Chad Versace 
>

Looks correct to me.

Reviewed-by: Paul Berry 


> ---
>  src/mesa/drivers/dri/i965/brw_blorp_clear.cpp | 35
> +--
>  1 file changed, 11 insertions(+), 24 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_blorp_clear.cpp
> b/src/mesa/drivers/dri/i965/brw_blorp_clear.cpp
> index 3f096b5..1904bde 100644
> --- a/src/mesa/drivers/dri/i965/brw_blorp_clear.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_blorp_clear.cpp
> @@ -265,30 +265,17 @@
> brw_blorp_clear_params::brw_blorp_clear_params(struct brw_context *brw,
>x_align *= 16;
>y_align *= 32;
>
> -  if (brw->is_haswell) {
> - /* From BSpec: 3D-Media-GPGPU Engine > 3D Pipeline > Pixel >
> Pixel
> -  * Backend > MCS Buffer for Render Target(s) [DevIVB+] > Table
> "Color
> -  * Clear of Non-MultiSampled Render Target Restrictions":
> -  *
> -  *   [IVB, VLVT, HSW]: Clear rectangle must be aligned to two
> times
> -  *   the number of pixels in the table shown below...  x_align,
> -  *   y_align values computed above are the relevant entries in
> the
> -  *   referred table.
> -  *
> -  * We apply the workaround to only Haswell because (a) we
> suspect that
> -  * is the only hardware where it is actually required and (b) we
> -  * haven't yet validated the workaround for the other hardware.
> -  */
> - x0 = ROUND_DOWN_TO(x0, 2 * x_align);
> - y0 = ROUND_DOWN_TO(y0, 2 * y_align);
> - x1 = ALIGN(x1, 2 * x_align);
> - y1 = ALIGN(y1, 2 * y_align);
> -  } else {
> - x0 = ROUND_DOWN_TO(x0,  x_align);
> - y0 = ROUND_DOWN_TO(y0, y_align);
> - x1 = ALIGN(x1, x_align);
> - y1 = ALIGN(y1, y_align);
> -  }
> +  /* From BSpec: 3D-Media-GPGPU Engine > 3D Pipeline > Pixel > Pixel
> +   * Backend > MCS Buffer for Render Target(s) [DevIVB+] > Table
> "Color
> +   * Clear of Non-MultiSampled Render Target Restrictions":
> +   *
> +   *   Clear rectangle must be aligned to two times the number of
> pixels in
> +   *   the table shown below due to 16x16 hashing across the slice.
> +   */
> +  x0 = ROUND_DOWN_TO(x0, 2 * x_align);
> +  y0 = ROUND_DOWN_TO(y0, 2 * y_align);
> +  x1 = ALIGN(x1, 2 * x_align);
> +  y1 = ALIGN(y1, 2 * y_align);
>
>/* From the Ivy Bridge PRM, Vol2 Part1 11.7 "MCS Buffer for Render
> * Target(s)", beneath the "Fast Color Clear" bullet (p327):
> --
> 1.8.4
>
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 2/3] mesa: Verify memory allocations success in _mesa_PushClientAttrib

[Juha-Pekka Heikkilä]

On Tue, Dec 3, 2013 at 6:42 PM, Brian Paul  wrote:
> On 12/02/2013 02:39 AM, Juha-Pekka Heikkila wrote:
>>
>> Check if any of the callocs fail and report it with _mesa_error
>> if needed.
>>
>> Signed-off-by: Juha-Pekka Heikkila 
>> ---
>>   src/mesa/main/attrib.c | 34 ++
>>   1 file changed, 30 insertions(+), 4 deletions(-)
>>
>> diff --git a/src/mesa/main/attrib.c b/src/mesa/main/attrib.c
>> index c9332bd..2418fb0 100644
>> --- a/src/mesa/main/attrib.c
>> +++ b/src/mesa/main/attrib.c
>> @@ -1488,6 +1488,12 @@ init_array_attrib_data(struct gl_context *ctx,
>>   {
>>  /* Get a non driver gl_array_object. */
>>  attrib->ArrayObj = CALLOC_STRUCT( gl_array_object );
>> +
>> +   if (attrib->ArrayObj == NULL) {
>> +  _mesa_error(ctx, GL_OUT_OF_MEMORY, "glPushClientAttrib");
>> +  return;
>> +   }
>> +
>
>
> This is good, but if CALLOC_STRUCT() fails we'd crash anyway because the
> following call to save_array_attrib() would dereference the null pointer.
> init_array_attrib_data() should probably return a true/false success/failure
> result so the following array attrib calls could be skipped.

I guess you are right, I'll change this to return boolean. This
function is called just from one place and I did add check that
attrib->ArrayObj is really in place to avoid the crash but that is
just silly hiding of functionality. My idea with checking
attrib->ArrayObj against NULL as detecting error was just to get out
from _mesa_PushClientAttrib easy and quick.

>
>>  _mesa_initialize_array_object(ctx, attrib->ArrayObj, 0);
>>   }
>>
>> @@ -1516,7 +1522,7 @@ _mesa_PushClientAttrib(GLbitfield mask)
>>  GET_CURRENT_CONTEXT(ctx);
>>
>>  if (ctx->ClientAttribStackDepth >= MAX_CLIENT_ATTRIB_STACK_DEPTH) {
>> -  _mesa_error( ctx, GL_STACK_OVERFLOW, "glPushClientAttrib" );
>> +  _mesa_error(ctx, GL_STACK_OVERFLOW, "glPushClientAttrib");
>> return;
>>  }
>>
>> @@ -1529,10 +1535,19 @@ _mesa_PushClientAttrib(GLbitfield mask)
>> struct gl_pixelstore_attrib *attr;
>> /* packing attribs */
>> attr = CALLOC_STRUCT( gl_pixelstore_attrib );
>> +  if (attr == NULL) {
>> + _mesa_error(ctx, GL_OUT_OF_MEMORY, "glPushClientAttrib");
>> + goto end;
>> +  }
>> copy_pixelstore(ctx, attr, &ctx->Pack);
>> save_attrib_data(&head, GL_CLIENT_PACK_BIT, attr);
>> /* unpacking attribs */
>> attr = CALLOC_STRUCT( gl_pixelstore_attrib );
>> +  if (attr == NULL) {
>> + _mesa_error(ctx, GL_OUT_OF_MEMORY, "glPushClientAttrib");
>> + goto end;
>> +  }
>> +
>> copy_pixelstore(ctx, attr, &ctx->Unpack);
>> save_attrib_data(&head, GL_CLIENT_UNPACK_BIT, attr);
>>  }
>> @@ -1540,13 +1555,24 @@ _mesa_PushClientAttrib(GLbitfield mask)
>>  if (mask & GL_CLIENT_VERTEX_ARRAY_BIT) {
>> struct gl_array_attrib *attr;
>> attr = CALLOC_STRUCT( gl_array_attrib );
>> +  if (attr == NULL) {
>> + _mesa_error(ctx, GL_OUT_OF_MEMORY, "glPushClientAttrib");
>> + goto end;
>> +  }
>> +
>> init_array_attrib_data(ctx, attr);
>> +  if (attr->ArrayObj == NULL) {
>> +  goto end;
>> +  }
>> +
>> save_array_attrib(ctx, attr, &ctx->Array);
>> save_attrib_data(&head, GL_CLIENT_VERTEX_ARRAY_BIT, attr);
>>  }
>> -
>> -   ctx->ClientAttribStack[ctx->ClientAttribStackDepth] = head;
>> -   ctx->ClientAttribStackDepth++;
>> +end:
>> +   if (head != NULL) {
>> +   ctx->ClientAttribStack[ctx->ClientAttribStackDepth] = head;
>> +   ctx->ClientAttribStackDepth++;
>> +   }
>>   }
>
>
> The rest looks OK.
>
> -Brian
>
>
> ___
> 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

[Mesa-dev] [Bug 72230] Unable to extract MesaLib-10.0.0.tar.{gz, bz2} with bsdtar

[bugzilla-daemon]

https://bugs.freedesktop.org/show_bug.cgi?id=72230

Emil Velikov  changed:

   What|Removed |Added

 Status|NEW |RESOLVED
 Resolution|--- |FIXED

--- Comment #3 from Emil Velikov  ---
Thanks, Patrick. Fix committed.

commit 507c2356e3caa3b7da98787ae2de96c7bca0089e
Author: Emil Velikov 
Date:   Mon Dec 2 19:42:51 2013 +

automake: include only one copy VERSION in tarball

The VERSION file is tracked by git (git ls-files), thus
adding it to EXTRA_FILES will result in a duplicate copy
within the final tarball.

-- 
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 72230] Unable to extract MesaLib-10.0.0.tar.{gz, bz2} with bsdtar

[bugzilla-daemon]

https://bugs.freedesktop.org/show_bug.cgi?id=72230

--- Comment #4 from Patrick Steinhardt  ---
Would it be possible to provide updated tarballs including this fix?

-- 
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] Nesa-dev now integrated with patchwork

[Carl Worth]

Carl Worth  writes:
> When a branch is pushed to the master git repo, a git hook will
> automatically search for the patch in the database, and if it is
> found,

...

At least, that was my intention.

I had a couple of permissions problem with the git hook, so it hasn't
actually been active. I fixed that now.

I also just manually fed the hook all of the commits that have been
received since the initial patch in patchwork's history. That process
updated 216 patches to the "Accepted" state.

So, you should all expect to see these hook messages now:

>   remote: I: patch # updated using rev .
...
>   remote: E: failed to find patch for rev .

and hopefully patchwork will be much more useful this way.

-Carl


pgppSfjbwDggc.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 3/4] radeonsi: Rearrange si_create_dsa_state

[Alex Deucher]

On Tue, Dec 3, 2013 at 3:33 PM, Andreas Hartmetz  wrote:
> Reduce scope of variables and divide the code more clearly into
> sections dealing with one thing.
> ---
>  src/gallium/drivers/radeonsi/si_state.c | 38 
> +++--
>  1 file changed, 22 insertions(+), 16 deletions(-)
>
> diff --git a/src/gallium/drivers/radeonsi/si_state.c 
> b/src/gallium/drivers/radeonsi/si_state.c
> index 7bae72a..24c9cf3 100644
> --- a/src/gallium/drivers/radeonsi/si_state.c
> +++ b/src/gallium/drivers/radeonsi/si_state.c
> @@ -706,25 +706,20 @@ static void *si_create_dsa_state(struct pipe_context 
> *ctx,
>  const struct pipe_depth_stencil_alpha_state 
> *state)
>  {
> struct si_state_dsa *dsa = CALLOC_STRUCT(si_state_dsa);
> -   struct si_pm4_state *pm4 = &dsa->pm4;
> -   unsigned db_depth_control;
> -   unsigned db_render_override, db_render_control;
> -   uint32_t db_stencil_control = 0;
> -

Please don't mix code and declarations, some compilers don't like that.

Alex

> if (dsa == NULL) {
> return NULL;
> }
>
> +   unsigned db_depth_control = 0;
> +
> +   /* stencil */
> +
> dsa->valuemask[0] = state->stencil[0].valuemask;
> dsa->valuemask[1] = state->stencil[1].valuemask;
> dsa->writemask[0] = state->stencil[0].writemask;
> dsa->writemask[1] = state->stencil[1].writemask;
>
> -   db_depth_control = S_028800_Z_ENABLE(state->depth.enabled) |
> -   S_028800_Z_WRITE_ENABLE(state->depth.writemask) |
> -   S_028800_ZFUNC(state->depth.func);
> -
> -   /* stencil */
> +   uint32_t db_stencil_control = 0;
> if (state->stencil[0].enabled) {
> db_depth_control |= S_028800_STENCIL_ENABLE(1);
> db_depth_control |= 
> S_028800_STENCILFUNC(state->stencil[0].func);
> @@ -740,8 +735,13 @@ static void *si_create_dsa_state(struct pipe_context 
> *ctx,
> db_stencil_control |= 
> S_02842C_STENCILZFAIL_BF(si_translate_stencil_op(state->stencil[1].zfail_op));
> }
> }
> +   unsigned db_render_override = 
> S_02800C_FORCE_HIS_ENABLE0(V_02800C_FORCE_DISABLE) |
> + 
> S_02800C_FORCE_HIS_ENABLE1(V_02800C_FORCE_DISABLE);
>
> /* alpha */
> +
> +   struct si_pm4_state *pm4 = &dsa->pm4;
> +
> if (state->alpha.enabled) {
> dsa->alpha_func = state->alpha.func;
> dsa->alpha_ref = state->alpha.ref_value;
> @@ -752,12 +752,19 @@ static void *si_create_dsa_state(struct pipe_context 
> *ctx,
> dsa->alpha_func = PIPE_FUNC_ALWAYS;
> }
>
> -   /* misc */
> -   db_render_control = 0;
> -   db_render_override = 
> S_02800C_FORCE_HIZ_ENABLE(V_02800C_FORCE_DISABLE) |
> -   S_02800C_FORCE_HIS_ENABLE0(V_02800C_FORCE_DISABLE) |
> -   S_02800C_FORCE_HIS_ENABLE1(V_02800C_FORCE_DISABLE);
> +   /* depth */
> +
> +   db_depth_control |= S_028800_Z_ENABLE(state->depth.enabled) |
> +   S_028800_Z_WRITE_ENABLE(state->depth.writemask) |
> +   S_028800_ZFUNC(state->depth.func);
> +   db_render_override |= 
> S_02800C_FORCE_HIZ_ENABLE(V_02800C_FORCE_DISABLE);
> +
> +   /* write remaining registers and return */
> +
> /* TODO db_render_override depends on query */
> +   dsa->db_render_override = db_render_override;
> +   unsigned db_render_control = 0;
> +
> si_pm4_set_reg(pm4, R_028020_DB_DEPTH_BOUNDS_MIN, 0x);
> si_pm4_set_reg(pm4, R_028024_DB_DEPTH_BOUNDS_MAX, 0x);
> si_pm4_set_reg(pm4, R_028028_DB_STENCIL_CLEAR, 0x);
> @@ -770,7 +777,6 @@ static void *si_create_dsa_state(struct pipe_context *ctx,
> si_pm4_set_reg(pm4, R_028AC0_DB_SRESULTS_COMPARE_STATE0, 0x0);
> si_pm4_set_reg(pm4, R_028AC4_DB_SRESULTS_COMPARE_STATE1, 0x0);
> si_pm4_set_reg(pm4, R_028AC8_DB_PRELOAD_CONTROL, 0x0);
> -   dsa->db_render_override = db_render_override;
>
> return dsa;
>  }
> --
> 1.8.3.2
>
> ___
> 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 4/4] radeonsi: Write htile state to SI hardware.

[Alex Deucher]

On Tue, Dec 3, 2013 at 3:33 PM, Andreas Hartmetz  wrote:
> ---
>  src/gallium/drivers/radeonsi/si_state.c | 28 +++-
>  1 file changed, 27 insertions(+), 1 deletion(-)
>
> diff --git a/src/gallium/drivers/radeonsi/si_state.c 
> b/src/gallium/drivers/radeonsi/si_state.c
> index 24c9cf3..7b1a6df 100644
> --- a/src/gallium/drivers/radeonsi/si_state.c
> +++ b/src/gallium/drivers/radeonsi/si_state.c
> @@ -757,7 +757,19 @@ static void *si_create_dsa_state(struct pipe_context 
> *ctx,
> db_depth_control |= S_028800_Z_ENABLE(state->depth.enabled) |
> S_028800_Z_WRITE_ENABLE(state->depth.writemask) |
> S_028800_ZFUNC(state->depth.func);
> -   db_render_override |= 
> S_02800C_FORCE_HIZ_ENABLE(V_02800C_FORCE_DISABLE);
> +
> +   struct r600_context *rctx = (struct r600_context *)ctx;
> +   struct pipe_surface *surf = rctx->framebuffer.zsbuf;
> +   struct r600_texture *rtex = 0;

Please don't mix code and declarations.

Alex


> +   if (surf) {
> +   rtex = (struct r600_texture*)surf->texture;
> +   }
> +   if (rtex && rtex->htile_buffer) {
> +   /* Force off means no force, DB_SHADER_CONTROL decides */
> +   db_render_override |= 
> S_02800C_FORCE_HIZ_ENABLE(V_02800C_FORCE_OFF);
> +   } else {
> +   db_render_override |= 
> S_02800C_FORCE_HIZ_ENABLE(V_02800C_FORCE_DISABLE);
> +   }
>
> /* write remaining registers and return */
>
> @@ -1842,9 +1854,21 @@ static void si_db(struct r600_context *rctx, struct 
> si_pm4_state *pm4,
> s_info |= S_028044_TILE_MODE_INDEX(tile_mode_index);
> }
>
> +   uint32_t db_htile_data_base = 0;
> +   uint32_t db_htile_surface = 0;
> +   /* use htile only for first level */
> +   if (rtex->htile_buffer && !level) {
> +   z_info |= S_028040_TILE_SURFACE_ENABLE(1);
> +   uint64_t va = r600_resource_va(&rctx->screen->b.b, 
> &rtex->htile_buffer->b.b);
> +   db_htile_data_base = va >> 8;
> +
> +   db_htile_surface = S_028ABC_FULL_CACHE(1);
> +   }
> +
> si_pm4_set_reg(pm4, R_028008_DB_DEPTH_VIEW,
>S_028008_SLICE_START(state->zsbuf->u.tex.first_layer) |
>S_028008_SLICE_MAX(state->zsbuf->u.tex.last_layer));
> +   si_pm4_set_reg(pm4, R_028014_DB_HTILE_DATA_BASE, db_htile_data_base);
>
> si_pm4_set_reg(pm4, R_02803C_DB_DEPTH_INFO, db_depth_info);
> si_pm4_set_reg(pm4, R_028040_DB_Z_INFO, z_info);
> @@ -1858,6 +1882,8 @@ static void si_db(struct r600_context *rctx, struct 
> si_pm4_state *pm4,
>
> si_pm4_set_reg(pm4, R_028058_DB_DEPTH_SIZE, 
> S_028058_PITCH_TILE_MAX(pitch));
> si_pm4_set_reg(pm4, R_02805C_DB_DEPTH_SLICE, 
> S_02805C_SLICE_TILE_MAX(slice));
> +
> +   si_pm4_set_reg(pm4, R_028ABC_DB_HTILE_SURFACE, db_htile_surface);
>  }
>
>  #define FILL_SREG(s0x, s0y, s1x, s1y, s2x, s2y, s3x, s3y)  \
> --
> 1.8.3.2
>
> ___
> 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/4] radeonsi: Rearrange si_create_dsa_state

[Matt Turner]

On Tue, Dec 3, 2013 at 2:56 PM, Alex Deucher  wrote:
> On Tue, Dec 3, 2013 at 3:33 PM, Andreas Hartmetz  wrote:
>> Reduce scope of variables and divide the code more clearly into
>> sections dealing with one thing.
>> ---
>>  src/gallium/drivers/radeonsi/si_state.c | 38 
>> +++--
>>  1 file changed, 22 insertions(+), 16 deletions(-)
>>
>> diff --git a/src/gallium/drivers/radeonsi/si_state.c 
>> b/src/gallium/drivers/radeonsi/si_state.c
>> index 7bae72a..24c9cf3 100644
>> --- a/src/gallium/drivers/radeonsi/si_state.c
>> +++ b/src/gallium/drivers/radeonsi/si_state.c
>> @@ -706,25 +706,20 @@ static void *si_create_dsa_state(struct pipe_context 
>> *ctx,
>>  const struct pipe_depth_stencil_alpha_state 
>> *state)
>>  {
>> struct si_state_dsa *dsa = CALLOC_STRUCT(si_state_dsa);
>> -   struct si_pm4_state *pm4 = &dsa->pm4;
>> -   unsigned db_depth_control;
>> -   unsigned db_render_override, db_render_control;
>> -   uint32_t db_stencil_control = 0;
>> -
>
> Please don't mix code and declarations, some compilers don't like that.

We avoid that in core Mesa because of MSVC, but I don't think radeonsi
will ever be compiled with MSVC?
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 2/4] radeon: Allocate htile buffer for SI in r600_texture.

[Alex Deucher]

On Tue, Dec 3, 2013 at 3:33 PM, Andreas Hartmetz  wrote:
> ---
>  src/gallium/drivers/radeon/r600_texture.c | 84 
> +--
>  1 file changed, 68 insertions(+), 16 deletions(-)
>
> diff --git a/src/gallium/drivers/radeon/r600_texture.c 
> b/src/gallium/drivers/radeon/r600_texture.c
> index 70f21bd..c91fbce 100644
> --- a/src/gallium/drivers/radeon/r600_texture.c
> +++ b/src/gallium/drivers/radeon/r600_texture.c
> @@ -457,12 +457,53 @@ void r600_texture_init_cmask(struct r600_common_screen 
> *rscreen,
> }
>  }
>
> -static void r600_texture_allocate_htile(struct r600_common_screen *rscreen,
> -   struct r600_texture *rtex)
> +static unsigned si_texture_htile_alloc_size(struct r600_common_screen 
> *rscreen,
> +   struct r600_texture *rtex)
> +{
> +   unsigned num_pipes = rscreen->tiling_info.num_channels;
> +
> +   unsigned cl_width;
> +   unsigned cl_height;
> +   switch (num_pipes) {
> +   case 2:
> +   cl_width = 32;
> +   cl_height = 32;
> +   break;
> +   case 4:
> +   cl_width = 64;
> +   cl_height = 32;
> +   break;
> +   case 8:
> +   cl_width = 64;
> +   cl_height = 64;
> +   break;
> +   case 16:
> +   cl_width = 128;
> +   cl_height = 64;
> +   break;
> +   default:
> +   assert(0);
> +   return 0;
> +   }
> +
> +   unsigned width = align(rtex->surface.npix_x, cl_width * 8);
> +   unsigned height = align(rtex->surface.npix_y, cl_height * 8);
> +
> +   unsigned slice_elements = (width * height) / (8 * 8);
> +   unsigned slice_bytes = slice_elements * 4;
> +
> +   unsigned pipe_interleave_bytes = rscreen->tiling_info.group_bytes;
> +   unsigned base_align = num_pipes * pipe_interleave_bytes;
> +   unsigned size = rtex->surface.array_size * align(slice_bytes, 
> base_align);
> +

Please don't mix code and declarations.

Alex

> +   return size;
> +}
> +
> +static unsigned r600_texture_htile_alloc_size(struct r600_common_screen 
> *rscreen,
> + struct r600_texture *rtex)
>  {
> unsigned sw = rtex->surface.level[0].nblk_x * rtex->surface.blk_w;
> unsigned sh = rtex->surface.level[0].nblk_y * rtex->surface.blk_h;
> -   unsigned htile_size;
> unsigned npipes = rscreen->info.r600_num_tile_pipes;
>
> /* XXX also use it for other texture targets */
> @@ -470,24 +511,39 @@ static void r600_texture_allocate_htile(struct 
> r600_common_screen *rscreen,
> rtex->resource.b.b.target != PIPE_TEXTURE_2D ||
> rtex->surface.level[0].nblk_x < 32 ||
> rtex->surface.level[0].nblk_y < 32) {
> -   return;
> +   return 0;
> }
>
> /* this alignment and htile size only apply to linear htile buffer */
> sw = align(sw, 16 << 3);
> sh = align(sh, npipes << 3);
> -   htile_size = (sw >> 3) * (sh >> 3) * 4;
> +   unsigned htile_size = (sw >> 3) * (sh >> 3) * 4;
> /* must be aligned with 2K * npipes */
> htile_size = align(htile_size, (2 << 10) * npipes);
> +   return htile_size;
> +}
> +
> +static void r600_texture_allocate_htile(struct r600_common_screen *rscreen,
> +   struct r600_texture *rtex)
> +{
> +   unsigned htile_size;
> +   if (rscreen->chip_class >= SI) {
> +   htile_size = si_texture_htile_alloc_size(rscreen, rtex);
> +   } else {
> +   htile_size = r600_texture_htile_alloc_size(rscreen, rtex);
> +   }
> +
> +   if (!htile_size)
> +   return;
>
> /* XXX don't allocate it separately */
> -   rtex->htile_buffer = (struct 
> r600_resource*)pipe_buffer_create(&rscreen->b, PIPE_BIND_CUSTOM,
> -  
> PIPE_USAGE_STATIC, htile_size);
> +   rtex->htile_buffer = (struct r600_resource*)
> +pipe_buffer_create(&rscreen->b, PIPE_BIND_CUSTOM,
> +   PIPE_USAGE_STATIC, 
> htile_size);
> if (rtex->htile_buffer == NULL) {
> /* this is not a fatal error as we can still keep rendering
> -* without htile buffer
> -*/
> -   R600_ERR("r600: failed to create bo for htile buffers\n");
> +* without htile buffer */
> +   R600_ERR("Failed to create buffer object for htile 
> buffer.\n");
> } else {
> r600_screen_clear_buffer(rscreen, &rtex->htile_buffer->b.b, 
> 0, htile_size, 0);
> }
> @@ -534,12 +590,8 @@ r600_texture_create_object(struct pipe_screen *screen,
> if (!(base->flags & (R600_RESOURCE_FLAG_TRANSFER |
> 

Re: [Mesa-dev] [PATCH 3/4] radeonsi: Rearrange si_create_dsa_state

[Alex Deucher]

On Tue, Dec 3, 2013 at 5:59 PM, Matt Turner  wrote:
> On Tue, Dec 3, 2013 at 2:56 PM, Alex Deucher  wrote:
>> On Tue, Dec 3, 2013 at 3:33 PM, Andreas Hartmetz  wrote:
>>> Reduce scope of variables and divide the code more clearly into
>>> sections dealing with one thing.
>>> ---
>>>  src/gallium/drivers/radeonsi/si_state.c | 38 
>>> +++--
>>>  1 file changed, 22 insertions(+), 16 deletions(-)
>>>
>>> diff --git a/src/gallium/drivers/radeonsi/si_state.c 
>>> b/src/gallium/drivers/radeonsi/si_state.c
>>> index 7bae72a..24c9cf3 100644
>>> --- a/src/gallium/drivers/radeonsi/si_state.c
>>> +++ b/src/gallium/drivers/radeonsi/si_state.c
>>> @@ -706,25 +706,20 @@ static void *si_create_dsa_state(struct pipe_context 
>>> *ctx,
>>>  const struct 
>>> pipe_depth_stencil_alpha_state *state)
>>>  {
>>> struct si_state_dsa *dsa = CALLOC_STRUCT(si_state_dsa);
>>> -   struct si_pm4_state *pm4 = &dsa->pm4;
>>> -   unsigned db_depth_control;
>>> -   unsigned db_render_override, db_render_control;
>>> -   uint32_t db_stencil_control = 0;
>>> -
>>
>> Please don't mix code and declarations, some compilers don't like that.
>
> We avoid that in core Mesa because of MSVC, but I don't think radeonsi
> will ever be compiled with MSVC?

It might be, plus there may be other compilers that don't like it.
Additionally, none of the other functions in radeonsi do it so it's a
departure from the overall coding style of the driver.

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

Re: [Mesa-dev] [PATCH 3/3] i965: Fix BRW_BATCH_STRUCT to specify RENDER_RING, not UNKNOWN_RING.

[Eric Anholt]

Kenneth Graunke  writes:

> I missed this in the boolean -> enum conversion.  C cheerfully casts
> false -> 0 -> UNKNOWN_RING.  On Gen4-5, this causes the render ring
> prelude hook to get called in the middle of the batch, which is crazy.
>
> BRW_BATCH_STRUCT is not used on Gen6+.
>
> Fixes regressions since 395a32717df494353703f3581edcd3ba380f16d6
> ("i965: Introduce an UNKNOWN_RING state.").
>
> Fixes "fips -v glxgears" on Ironlake.
>
> Signed-off-by: Kenneth Graunke 
> Cc: Eric Anholt 

These are:

Reviewed-by: Eric Anholt 


pgp62HQ6SDG7p.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 7/9] mesa: Fill out ARB_texture_view entry points

[Brian Paul]

On 12/03/2013 02:56 PM, Courtney Goeltzenleuchter wrote:

Hi Brian,

I've made all the recommended changes.

I also added one, a test that the Driver.TextureView != NULL before
calling. This lets me test the non DD functionality with the
MESA_EXTENSION_OVERRIDE=+GL_ARB_texture_view

I've also rebased to latest master (no changes required). Whole stream
can be found at:
https://github.com/courtney-lunarg/mesa/tree/texture_view-rc6


Let me know if you need me to post the patches to the mail list or not.


The series LGTM.  Thanks.  Reviewed-by: Brian Paul 

Do you need someone to push to master for you?

-Brian

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

Re: [Mesa-dev] [PATCH 7/9] mesa: Fill out ARB_texture_view entry points

[Courtney Goeltzenleuchter]

On Tue, Dec 3, 2013 at 4:28 PM, Brian Paul  wrote:

> On 12/03/2013 02:56 PM, Courtney Goeltzenleuchter wrote:
>
>> Hi Brian,
>>
>> I've made all the recommended changes.
>>
>> I also added one, a test that the Driver.TextureView != NULL before
>> calling. This lets me test the non DD functionality with the
>> MESA_EXTENSION_OVERRIDE=+GL_ARB_texture_view
>>
>> I've also rebased to latest master (no changes required). Whole stream
>> can be found at:
>> https://github.com/courtney-lunarg/mesa/tree/texture_view-rc6
>> > //github.com/courtney-lunarg/mesa/tree/texture_view-rc6&k=
>> oIvRg1%2BdGAgOoM1BIlLLqw%3D%3D%0A&r=lGQMzzTgII0I7jefp2FHq7WtZ%
>> 2BTLs8wadB%2BiIj9xpBY%3D%0A&m=StSJ4O%2Fa%2B9Zp%2FeTtJIDzJ1NxrKtDXrENaO%
>> 2FoFA9gY1k%3D%0A&s=0439e3fc50d195734c13b4c5e22024
>> f034c27b7e657c8f79e08993b98f117518>
>>
>>
>> Let me know if you need me to post the patches to the mail list or not.
>>
>
> The series LGTM.  Thanks.  Reviewed-by: Brian Paul 
>
> Do you need someone to push to master for you?


Yes please!


>
>
> -Brian
>
>


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

Re: [Mesa-dev] [PATCH 2/3] mesa: Verify memory allocations success in _mesa_PushClientAttrib

[Ian Romanick]

On 12/03/2013 01:24 PM, Juha-Pekka Heikkilä wrote:
> On Tue, Dec 3, 2013 at 6:42 PM, Brian Paul  wrote:
>> On 12/02/2013 02:39 AM, Juha-Pekka Heikkila wrote:
>>>
>>> Check if any of the callocs fail and report it with _mesa_error
>>> if needed.
>>>
>>> Signed-off-by: Juha-Pekka Heikkila 
>>> ---
>>>   src/mesa/main/attrib.c | 34 ++
>>>   1 file changed, 30 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/src/mesa/main/attrib.c b/src/mesa/main/attrib.c
>>> index c9332bd..2418fb0 100644
>>> --- a/src/mesa/main/attrib.c
>>> +++ b/src/mesa/main/attrib.c
>>> @@ -1488,6 +1488,12 @@ init_array_attrib_data(struct gl_context *ctx,
>>>   {
>>>  /* Get a non driver gl_array_object. */
>>>  attrib->ArrayObj = CALLOC_STRUCT( gl_array_object );
>>> +
>>> +   if (attrib->ArrayObj == NULL) {
>>> +  _mesa_error(ctx, GL_OUT_OF_MEMORY, "glPushClientAttrib");
>>> +  return;
>>> +   }
>>> +
>>
>>
>> This is good, but if CALLOC_STRUCT() fails we'd crash anyway because the
>> following call to save_array_attrib() would dereference the null pointer.
>> init_array_attrib_data() should probably return a true/false success/failure
>> result so the following array attrib calls could be skipped.
> 
> I guess you are right, I'll change this to return boolean. This

Pre-emptive review comment: Use bool / true / false from stdbool.h. :)

> function is called just from one place and I did add check that
> attrib->ArrayObj is really in place to avoid the crash but that is
> just silly hiding of functionality. My idea with checking
> attrib->ArrayObj against NULL as detecting error was just to get out
> from _mesa_PushClientAttrib easy and quick.
> 
>>>  _mesa_initialize_array_object(ctx, attrib->ArrayObj, 0);
>>>   }
>>>
>>> @@ -1516,7 +1522,7 @@ _mesa_PushClientAttrib(GLbitfield mask)
>>>  GET_CURRENT_CONTEXT(ctx);
>>>
>>>  if (ctx->ClientAttribStackDepth >= MAX_CLIENT_ATTRIB_STACK_DEPTH) {
>>> -  _mesa_error( ctx, GL_STACK_OVERFLOW, "glPushClientAttrib" );
>>> +  _mesa_error(ctx, GL_STACK_OVERFLOW, "glPushClientAttrib");
>>> return;
>>>  }
>>>
>>> @@ -1529,10 +1535,19 @@ _mesa_PushClientAttrib(GLbitfield mask)
>>> struct gl_pixelstore_attrib *attr;
>>> /* packing attribs */
>>> attr = CALLOC_STRUCT( gl_pixelstore_attrib );
>>> +  if (attr == NULL) {
>>> + _mesa_error(ctx, GL_OUT_OF_MEMORY, "glPushClientAttrib");
>>> + goto end;
>>> +  }
>>> copy_pixelstore(ctx, attr, &ctx->Pack);
>>> save_attrib_data(&head, GL_CLIENT_PACK_BIT, attr);
>>> /* unpacking attribs */
>>> attr = CALLOC_STRUCT( gl_pixelstore_attrib );
>>> +  if (attr == NULL) {
>>> + _mesa_error(ctx, GL_OUT_OF_MEMORY, "glPushClientAttrib");
>>> + goto end;
>>> +  }
>>> +
>>> copy_pixelstore(ctx, attr, &ctx->Unpack);
>>> save_attrib_data(&head, GL_CLIENT_UNPACK_BIT, attr);
>>>  }
>>> @@ -1540,13 +1555,24 @@ _mesa_PushClientAttrib(GLbitfield mask)
>>>  if (mask & GL_CLIENT_VERTEX_ARRAY_BIT) {
>>> struct gl_array_attrib *attr;
>>> attr = CALLOC_STRUCT( gl_array_attrib );
>>> +  if (attr == NULL) {
>>> + _mesa_error(ctx, GL_OUT_OF_MEMORY, "glPushClientAttrib");
>>> + goto end;
>>> +  }
>>> +
>>> init_array_attrib_data(ctx, attr);
>>> +  if (attr->ArrayObj == NULL) {
>>> +  goto end;
>>> +  }
>>> +
>>> save_array_attrib(ctx, attr, &ctx->Array);
>>> save_attrib_data(&head, GL_CLIENT_VERTEX_ARRAY_BIT, attr);
>>>  }
>>> -
>>> -   ctx->ClientAttribStack[ctx->ClientAttribStackDepth] = head;
>>> -   ctx->ClientAttribStackDepth++;
>>> +end:
>>> +   if (head != NULL) {
>>> +   ctx->ClientAttribStack[ctx->ClientAttribStackDepth] = head;
>>> +   ctx->ClientAttribStackDepth++;
>>> +   }
>>>   }
>>
>>
>> The rest looks OK.
>>
>> -Brian
>>
> ___
> 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 4/4] radeonsi: Write htile state to SI hardware.

[Michel Dänzer]

On Die, 2013-12-03 at 21:33 +0100, Andreas Hartmetz wrote:
> 
> diff --git a/src/gallium/drivers/radeonsi/si_state.c 
> b/src/gallium/drivers/radeonsi/si_state.c
> index 24c9cf3..7b1a6df 100644
> --- a/src/gallium/drivers/radeonsi/si_state.c
> +++ b/src/gallium/drivers/radeonsi/si_state.c
> @@ -1842,9 +1854,21 @@ static void si_db(struct r600_context *rctx, struct 
> si_pm4_state *pm4,
>   s_info |= S_028044_TILE_MODE_INDEX(tile_mode_index);
>   }
>  
> + uint32_t db_htile_data_base = 0;
> + uint32_t db_htile_surface = 0;
> + /* use htile only for first level */
> + if (rtex->htile_buffer && !level) {
> + z_info |= S_028040_TILE_SURFACE_ENABLE(1);
> + uint64_t va = r600_resource_va(&rctx->screen->b.b, 
> &rtex->htile_buffer->b.b);
> + db_htile_data_base = va >> 8;
> +
> + db_htile_surface = S_028ABC_FULL_CACHE(1);
> + }
> +
>   si_pm4_set_reg(pm4, R_028008_DB_DEPTH_VIEW,
>  S_028008_SLICE_START(state->zsbuf->u.tex.first_layer) |
>  S_028008_SLICE_MAX(state->zsbuf->u.tex.last_layer));
> + si_pm4_set_reg(pm4, R_028014_DB_HTILE_DATA_BASE, db_htile_data_base);
>  
>   si_pm4_set_reg(pm4, R_02803C_DB_DEPTH_INFO, db_depth_info);
>   si_pm4_set_reg(pm4, R_028040_DB_Z_INFO, z_info);
> @@ -1858,6 +1882,8 @@ static void si_db(struct r600_context *rctx, struct 
> si_pm4_state *pm4,
>  
>   si_pm4_set_reg(pm4, R_028058_DB_DEPTH_SIZE, 
> S_028058_PITCH_TILE_MAX(pitch));
>   si_pm4_set_reg(pm4, R_02805C_DB_DEPTH_SLICE, 
> S_02805C_SLICE_TILE_MAX(slice));
> +
> + si_pm4_set_reg(pm4, R_028ABC_DB_HTILE_SURFACE, db_htile_surface);
>  }

Do the R_028014_DB_HTILE_DATA_BASE and R_028ABC_DB_HTILE_SURFACE
registers need to be written when S_028040_TILE_SURFACE_ENABLE is not
enabled in R_028040_DB_Z_INFO?


BTW, I agree with Alex about mixing code and declarations.


-- 
Earthling Michel Dänzer|  http://www.amd.com
Libre software enthusiast  |Mesa and X developer

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

Re: [Mesa-dev] [PATCH 03/15] i965/cfg: Rework to make IF & ELSE blocks flow into ENDIF.

[Matt Turner]

On Mon, Dec 2, 2013 at 10:40 AM, Matt Turner  wrote:
> Previously we made the basic block following an ENDIF instruction a
> successor of the basic blocks ending with IF and ELSE. The PRM says that
> IF and ELSE instructions jump *to* the ENDIF, rather than over it.
>
> This should be immaterial to dataflow analysis, except for if, break,
> endif sequences:
>
>START B1 <-B0 <-B9
> 0x0100: cmp.g.f0(8) nullg15<8,8,1>F g4<0,1,0>F
> 0x0110: (+f0) if(8) 0 0 null0xUD
>END B1 ->B2 ->B4
>START B2 <-B1
>break
> 0x0120: break(8) 0 0null0D
>END B2 ->B10
>START B3
> 0x0130: endif(8) 2  null0x0002UD
>END B3 ->B4
>
> The ENDIF block would have no parents, so dataflow analysis would
> generate incorrect results, preventing copy propagation from eliminating
> some instructions.
>
> This patch changes the CFG to make ENDIF start rather than end basic
> blocks, so that it can be the jump target of the IF and ELSE
> instructions.
>
> It helps three programs (including two fs8/fs16 pairs) and hurts a
> single fs8/fs16 pair.
>
> total instructions in shared programs: 1561126 -> 1561066 (-0.00%)
> instructions in affected programs: 983 -> 923 (-6.10%)
>
> More importantly, it allows copy propagation to handle more cases.
> Disabling the register_coalesce() pass before this patch hurts 58
> programs, while afterward it only hurts 11 programs.
> ---

Eric mentioned on IRC that he was concerned about this patch and that
it may be papering over some strange behavior.

The background is that while investigating a regression caused by my
value numbering pass, I found two raw MOVs that should have been copy
propagated. The shader (sam3/101.frag) has a loop with an
if/break/endif sequence. In attempting to reduce the amount of effort
to debug copy propagation, I applied my predicated break patch and the
MOVs were properly copy propagated and removed.

In fact, this patch is an attempt to fix the real problem before
committing the predicated break pass which itself would paper over the
problem. :)

The attached diff is the output of

diff --git a/src/mesa/drivers/dri/i965/brw_fs_copy_propagation.cpp
b/src/mesa/drivers/dri/i965/brw_fs_copy_propagation.cpp
index 26bac94..ff68519 100644
--- a/src/mesa/drivers/dri/i965/brw_fs_copy_propagation.cpp
+++ b/src/mesa/drivers/dri/i965/brw_fs_copy_propagation.cpp
@@ -560,6 +560,9 @@ fs_visitor::opt_copy_propagate()
   progress = opt_copy_propagate_local(mem_ctx, block, in_acp) || progress;
}

+   cfg.dump(this);
+   dataflow.dump_block_data();
+
for (int i = 0; i < cfg.num_blocks; i++)
   delete [] out_acp[i];
ralloc_free(mem_ctx);

during the first call of opt_copy_propagate(), before and after this patch.

The diff shows that B6 in the before case has no parents, and its
livein/liveout sets are empty. If I understand the dataflow algorithm
correctly, this is a problem. liveout is calculated by ANDing livein,
and if the block incorrectly has no parents, livein will be 0 rather
than ~0. Its liveout is then used to compute the livein of other
blocks, so the problem cascades. I think this can be seen looking at
B6's successors B7 and (indirectly through B7) B4 which have empty
livein and empty or wrong liveout.

We could avoid this problem by modifying the dataflow analysis code to
assign only B0 an empty livein set, but *that* feels like papering it
over. :)

Another concern was about ENDIF starting, rather than ending basic
blocks. I think this concern has been alleviated. ENDIF doesn't jump
itself (except to the next instruction; and in one other case that we
don't do) and is rather the jump target of the IF and ELSE
instructions, so it's correct to have it start a basic block.
--- dataflow-before 2013-12-03 18:16:25.069616345 -0800
+++ dataflow-after  2013-12-03 18:17:32.146610248 -0800
@@ -1,215 +1,211 @@
 START B0
 0: mov vgrf5, u0, (null), (null),  
 1: mov vgrf103, u1, (null), (null),  
 2: mov vgrf104, u2, (null), (null),  
 3: mov vgrf105, u3, (null), (null),  
 4: mov vgrf2, u2, (null), (null),  
 5: mov vgrf4, u4, (null), (null),  
 6: mov vgrf100, u5, (null), (null),  
 7: mov vgrf101, u6, (null), (null),  
 8: mov vgrf102, u7, (null), (null),  
 9: mov vgrf3, u4, (null), (null),  
10: mov vgrf18, vgrf1, (null), (null),  
11: mov vgrf109, vgrf99, (null), (null),  
12: mov vgrf19, vgrf1, (null), (null),  
13: mov vgrf19+1, vgrf99, (null), (null),  
14: tex vgrf17, vgrf19, (null), (null),  
15: mov vgrf20, 65504.00f, (null), (null),  
16: sel.cmod vgrf13, vgrf17, 65504.00f, (null),  
17: mov vgrf21, 65504.00f, (null), (null),  
18: sel.cmod vgrf14, vgrf17+1, 65504.00f, (null),  
19: mov vgrf22, 65504.00f, (null), (null),  
20: sel.cmod vgrf15, vgrf17+2, 65504.00f, (null),  
21: mov v

Re: [Mesa-dev] [PATCH] winsys/radeon: set/get the scanout flag with the tiling ioctls

[Michel Dänzer]

On Fre, 2013-11-29 at 19:08 +0100, Marek Olšák wrote:
> From: Marek Olšák 
> 
> If we assume that all buffers allocated by the DDX are scanout, a new flag
> that says "this is not scanout" has to be added to support the non-scanout
> buffers and maintain backward compatibility.
> 
> This fixes bad rendering on Wayland.
> 
> The flag is defined as:
>   #define RADEON_TILING_R600_NO_SCANOUT   RADEON_TILING_SWAP_16BIT
> 
> AFAIK, RADEON_TILING_SWAP_16BIT is not used on SI.

Is it used on >= R600? If not, might it make sense to use the new flag
as of R600?

Either way though,

Reviewed-by: Michel Dänzer 


-- 
Earthling Michel Dänzer|  http://www.amd.com
Libre software enthusiast  |Mesa and X developer

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

[Mesa-dev] [PATCH] radeonsi: Remove some stale XXX / FIXME comments

[Michel Dänzer]

From: Michel Dänzer 

Signed-off-by: Michel Dänzer 
---
 src/gallium/drivers/radeonsi/radeonsi_pipe.c   | 1 -
 src/gallium/drivers/radeonsi/radeonsi_shader.c | 5 +
 2 files changed, 1 insertion(+), 5 deletions(-)

diff --git a/src/gallium/drivers/radeonsi/radeonsi_pipe.c 
b/src/gallium/drivers/radeonsi/radeonsi_pipe.c
index 0242a03..4aaa88f 100644
--- a/src/gallium/drivers/radeonsi/radeonsi_pipe.c
+++ b/src/gallium/drivers/radeonsi/radeonsi_pipe.c
@@ -392,7 +392,6 @@ static int r600_get_param(struct pipe_screen* pscreen, enum 
pipe_cap param)
 
/* Render targets. */
case PIPE_CAP_MAX_RENDER_TARGETS:
-   /* FIXME some r6xx are buggy and can only do 4 */
return 8;
 
case PIPE_CAP_MAX_VIEWPORTS:
diff --git a/src/gallium/drivers/radeonsi/radeonsi_shader.c 
b/src/gallium/drivers/radeonsi/radeonsi_shader.c
index 96cc1aa..88825bb 100644
--- a/src/gallium/drivers/radeonsi/radeonsi_shader.c
+++ b/src/gallium/drivers/radeonsi/radeonsi_shader.c
@@ -281,7 +281,6 @@ static void declare_input_fs(
attr_number = lp_build_const_int32(gallivm,
   
shader->input[input_index].param_offset);
 
-   /* XXX: Handle all possible interpolation modes */
switch (decl->Interp.Interpolate) {
case TGSI_INTERPOLATE_COLOR:
if (si_shader_ctx->shader->key.ps.flatshade) {
@@ -875,7 +874,7 @@ static void si_llvm_emit_streamout(struct si_shader_context 
*shader)
lp_build_endif(&if_ctx);
 }
 
-/* XXX: This is partially implemented for VS only at this point.  It is not 
complete */
+
 static void si_llvm_emit_epilogue(struct lp_build_tgsi_context * bld_base)
 {
struct si_shader_context * si_shader_ctx = si_shader_context(bld_base);
@@ -1222,8 +1221,6 @@ handle_semantic:
   
LLVMVoidTypeInContext(base->gallivm->context),
   last_args, 9);
}
-/* XXX: Look up what this function does */
-/* ctx->shader->output[i].spi_sid = 
r600_spi_sid(&ctx->shader->output[i]);*/
 }
 
 static const struct lp_build_tgsi_action txf_action;
-- 
1.8.4.3

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

Re: [Mesa-dev] [PATCH 00/10] Sharing r600g glMapBuffer optimizations with radeonsi

[Michel Dänzer]

On Fre, 2013-11-29 at 18:55 +0100, Marek Olšák wrote:
> This series moves the r600_buffer.c files from both drivers to the shared 
> directory gallium/drivers/radeon, and implements what's missing for radeonsi 
> to make sharing the code possible.
> 
> This improves Valve's Team Fortress 2 performance by 75%.
> 
> Before: 20 fps
> After: 35 fps

Nice!

The series is

Reviewed-by: Michel Dänzer 


-- 
Earthling Michel Dänzer|  http://www.amd.com
Libre software enthusiast  |Mesa and X developer

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

[Mesa-dev] [PATCH V2] Modified _mesa_align_free to have consistent behavior when dealing with NULL memory address.

[Siavash Eliasi]

Revision 2:
- Avoiding compile error on MSVC and possible warnings on other compilers.
- Added comment regards passing NULL pointer being safe.
---
 src/mesa/main/imports.c| 14 +-
 src/mesa/math/m_matrix.c   | 13 +
 src/mesa/program/prog_parameter.c  |  3 +--
 src/mesa/state_tracker/st_cb_texture.c |  6 ++
 src/mesa/swrast/s_texture.c|  7 +++
 src/mesa/tnl/t_vertex.c|  6 ++
 src/mesa/vbo/vbo_exec_api.c|  9 -
 7 files changed, 26 insertions(+), 32 deletions(-)

diff --git a/src/mesa/main/imports.c b/src/mesa/main/imports.c
index 277e947..4afe156 100644
--- a/src/mesa/main/imports.c
+++ b/src/mesa/main/imports.c
@@ -168,6 +168,8 @@ _mesa_align_calloc(size_t bytes, unsigned long alignment)
  * \param ptr pointer to the memory to be freed.
  * The actual address to free is stored in the word immediately before the
  * address the client sees.
+ * Note that it is legal to pass NULL pointer to this function and will be
+ * handled accordingly.
  */
 void
 _mesa_align_free(void *ptr)
@@ -177,9 +179,11 @@ _mesa_align_free(void *ptr)
 #elif defined(_WIN32) && defined(_MSC_VER)
_aligned_free(ptr);
 #else
-   void **cubbyHole = (void **) ((char *) ptr - sizeof(void *));
-   void *realAddr = *cubbyHole;
-   free(realAddr);
+   if (ptr) {
+  void **cubbyHole = (void **) ((char *) ptr - sizeof(void *));
+  void *realAddr = *cubbyHole;
+  free(realAddr);
+   }
 #endif /* defined(HAVE_POSIX_MEMALIGN) */
 }
 
@@ -199,8 +203,8 @@ _mesa_align_realloc(void *oldBuffer, size_t oldSize, size_t 
newSize,
if (newBuf && oldBuffer && copySize > 0) {
   memcpy(newBuf, oldBuffer, copySize);
}
-   if (oldBuffer)
-  _mesa_align_free(oldBuffer);
+
+   _mesa_align_free(oldBuffer);
return newBuf;
 #endif
 }
diff --git a/src/mesa/math/m_matrix.c b/src/mesa/math/m_matrix.c
index 2902315..274f969 100644
--- a/src/mesa/math/m_matrix.c
+++ b/src/mesa/math/m_matrix.c
@@ -1488,14 +1488,11 @@ _math_matrix_ctr( GLmatrix *m )
 void
 _math_matrix_dtr( GLmatrix *m )
 {
-   if (m->m) {
-  _mesa_align_free( m->m );
-  m->m = NULL;
-   }
-   if (m->inv) {
-  _mesa_align_free( m->inv );
-  m->inv = NULL;
-   }
+   _mesa_align_free( m->m );
+   m->m = NULL;
+
+   _mesa_align_free( m->inv );
+   m->inv = NULL;
 }
 
 /*@}*/
diff --git a/src/mesa/program/prog_parameter.c 
b/src/mesa/program/prog_parameter.c
index 4d9cf08..54531d2 100644
--- a/src/mesa/program/prog_parameter.c
+++ b/src/mesa/program/prog_parameter.c
@@ -83,8 +83,7 @@ _mesa_free_parameter_list(struct gl_program_parameter_list 
*paramList)
   free((void *)paramList->Parameters[i].Name);
}
free(paramList->Parameters);
-   if (paramList->ParameterValues)
-  _mesa_align_free(paramList->ParameterValues);
+   _mesa_align_free(paramList->ParameterValues);
free(paramList);
 }
 
diff --git a/src/mesa/state_tracker/st_cb_texture.c 
b/src/mesa/state_tracker/st_cb_texture.c
index faa9ee3..f33d3cf 100644
--- a/src/mesa/state_tracker/st_cb_texture.c
+++ b/src/mesa/state_tracker/st_cb_texture.c
@@ -175,10 +175,8 @@ st_FreeTextureImageBuffer(struct gl_context *ctx,
   pipe_resource_reference(&stImage->pt, NULL);
}
 
-   if (stImage->TexData) {
-  _mesa_align_free(stImage->TexData);
-  stImage->TexData = NULL;
-   }
+   _mesa_align_free(stImage->TexData);
+   stImage->TexData = NULL;
 }
 
 
diff --git a/src/mesa/swrast/s_texture.c b/src/mesa/swrast/s_texture.c
index 27803c5..c08a4e9 100644
--- a/src/mesa/swrast/s_texture.c
+++ b/src/mesa/swrast/s_texture.c
@@ -164,10 +164,9 @@ _swrast_free_texture_image_buffer(struct gl_context *ctx,
   struct gl_texture_image *texImage)
 {
struct swrast_texture_image *swImage = swrast_texture_image(texImage);
-   if (swImage->Buffer) {
-  _mesa_align_free(swImage->Buffer);
-  swImage->Buffer = NULL;
-   }
+
+   _mesa_align_free(swImage->Buffer);
+   swImage->Buffer = NULL;
 
free(swImage->ImageSlices);
swImage->ImageSlices = NULL;
diff --git a/src/mesa/tnl/t_vertex.c b/src/mesa/tnl/t_vertex.c
index c7a745e..8c4195e 100644
--- a/src/mesa/tnl/t_vertex.c
+++ b/src/mesa/tnl/t_vertex.c
@@ -546,10 +546,8 @@ void _tnl_free_vertices( struct gl_context *ctx )
   struct tnl_clipspace *vtx = GET_VERTEX_STATE(ctx);
   struct tnl_clipspace_fastpath *fp, *tmp;
 
-  if (vtx->vertex_buf) {
- _mesa_align_free(vtx->vertex_buf);
- vtx->vertex_buf = NULL;
-  }
+  _mesa_align_free(vtx->vertex_buf);
+  vtx->vertex_buf = NULL;
 
   for (fp = vtx->fastpath ; fp ; fp = tmp) {
  tmp = fp->next;
diff --git a/src/mesa/vbo/vbo_exec_api.c b/src/mesa/vbo/vbo_exec_api.c
index 600398c..f3c41e0 100644
--- a/src/mesa/vbo/vbo_exec_api.c
+++ b/src/mesa/vbo/vbo_exec_api.c
@@ -989,11 +989,10 @@ void vbo_use_buffer_objects(struct gl_context *ctx)
 
/* Make sure this func is only used once */
assert(exec->v

Re: [Mesa-dev] [PATCH] Modified _mesa_align_free to have consistent behavior when dealing with NULL memory address.

[Siavash Eliasi]

On 12/03/2013 09:48 PM, Brian Paul wrote:

On 11/26/2013 10:59 PM, Siavash Eliasi wrote:

---
  src/mesa/main/imports.c|  7 +--
  src/mesa/math/m_matrix.c   | 13 +
  src/mesa/program/prog_parameter.c  |  3 +--
  src/mesa/state_tracker/st_cb_texture.c |  6 ++
  src/mesa/swrast/s_texture.c|  7 +++
  src/mesa/tnl/t_vertex.c|  6 ++
  src/mesa/vbo/vbo_exec_api.c|  9 -
  7 files changed, 22 insertions(+), 29 deletions(-)

diff --git a/src/mesa/main/imports.c b/src/mesa/main/imports.c
index 277e947..edfc0d1 100644
--- a/src/mesa/main/imports.c
+++ b/src/mesa/main/imports.c
@@ -177,6 +177,9 @@ _mesa_align_free(void *ptr)
  #elif defined(_WIN32) && defined(_MSC_VER)
 _aligned_free(ptr);
  #else
+   if (ptr == NULL)
+  return;
+


We can't have code before declarations like this (MSVC will error and 
some other compilers warn depending on the C variant being targeted.)


So, how about:

   if (ptr) {
  void **cubbyHole = (void **) ((char *) ptr - sizeof(void *));
  void *realAddr = *cubbyHole;
  free(realAddr);
   }

Maybe update the comment on the function to say that passing a NULL 
pointer is legal.


Thanks for reviewing, all suggested changes are done. Here is the V2 
patch containing those changes:


[PATCH V2] Modified _mesa_align_free to have consistent behavior when 
dealing with NULL memory address

http://lists.freedesktop.org/archives/mesa-dev/2013-December/049650.html



The rest looks fine.

With that fixed: Reviewed-by: Brian Paul 

Do you need someone to push patches for you?


Yes please. I don't have write access.


Best regards,
Siavash Eliasi.





 void **cubbyHole = (void **) ((char *) ptr - sizeof(void *));
 void *realAddr = *cubbyHole;
 free(realAddr);
@@ -199,8 +202,8 @@ _mesa_align_realloc(void *oldBuffer, size_t 
oldSize, size_t newSize,

 if (newBuf && oldBuffer && copySize > 0) {
memcpy(newBuf, oldBuffer, copySize);
 }
-   if (oldBuffer)
-  _mesa_align_free(oldBuffer);
+
+   _mesa_align_free(oldBuffer);
 return newBuf;
  #endif
  }
diff --git a/src/mesa/math/m_matrix.c b/src/mesa/math/m_matrix.c
index 2902315..274f969 100644
--- a/src/mesa/math/m_matrix.c
+++ b/src/mesa/math/m_matrix.c
@@ -1488,14 +1488,11 @@ _math_matrix_ctr( GLmatrix *m )
  void
  _math_matrix_dtr( GLmatrix *m )
  {
-   if (m->m) {
-  _mesa_align_free( m->m );
-  m->m = NULL;
-   }
-   if (m->inv) {
-  _mesa_align_free( m->inv );
-  m->inv = NULL;
-   }
+   _mesa_align_free( m->m );
+   m->m = NULL;
+
+   _mesa_align_free( m->inv );
+   m->inv = NULL;
  }

  /*@}*/
diff --git a/src/mesa/program/prog_parameter.c 
b/src/mesa/program/prog_parameter.c

index 4d9cf08..54531d2 100644
--- a/src/mesa/program/prog_parameter.c
+++ b/src/mesa/program/prog_parameter.c
@@ -83,8 +83,7 @@ _mesa_free_parameter_list(struct 
gl_program_parameter_list *paramList)

free((void *)paramList->Parameters[i].Name);
 }
 free(paramList->Parameters);
-   if (paramList->ParameterValues)
-  _mesa_align_free(paramList->ParameterValues);
+   _mesa_align_free(paramList->ParameterValues);
 free(paramList);
  }

diff --git a/src/mesa/state_tracker/st_cb_texture.c 
b/src/mesa/state_tracker/st_cb_texture.c

index faa9ee3..f33d3cf 100644
--- a/src/mesa/state_tracker/st_cb_texture.c
+++ b/src/mesa/state_tracker/st_cb_texture.c
@@ -175,10 +175,8 @@ st_FreeTextureImageBuffer(struct gl_context *ctx,
pipe_resource_reference(&stImage->pt, NULL);
 }

-   if (stImage->TexData) {
-  _mesa_align_free(stImage->TexData);
-  stImage->TexData = NULL;
-   }
+   _mesa_align_free(stImage->TexData);
+   stImage->TexData = NULL;
  }


diff --git a/src/mesa/swrast/s_texture.c b/src/mesa/swrast/s_texture.c
index 27803c5..c08a4e9 100644
--- a/src/mesa/swrast/s_texture.c
+++ b/src/mesa/swrast/s_texture.c
@@ -164,10 +164,9 @@ _swrast_free_texture_image_buffer(struct 
gl_context *ctx,

struct gl_texture_image *texImage)
  {
 struct swrast_texture_image *swImage = 
swrast_texture_image(texImage);

-   if (swImage->Buffer) {
-  _mesa_align_free(swImage->Buffer);
-  swImage->Buffer = NULL;
-   }
+
+   _mesa_align_free(swImage->Buffer);
+   swImage->Buffer = NULL;

 free(swImage->ImageSlices);
 swImage->ImageSlices = NULL;
diff --git a/src/mesa/tnl/t_vertex.c b/src/mesa/tnl/t_vertex.c
index c7a745e..8c4195e 100644
--- a/src/mesa/tnl/t_vertex.c
+++ b/src/mesa/tnl/t_vertex.c
@@ -546,10 +546,8 @@ void _tnl_free_vertices( struct gl_context *ctx )
struct tnl_clipspace *vtx = GET_VERTEX_STATE(ctx);
struct tnl_clipspace_fastpath *fp, *tmp;

-  if (vtx->vertex_buf) {
- _mesa_align_free(vtx->vertex_buf);
- vtx->vertex_buf = NULL;
-  }
+  _mesa_align_free(vtx->vertex_buf);
+  vtx->vertex_buf = NULL;

for (fp = vtx->fastpath ; fp ; fp = tmp) {
   tmp = fp->n