Re: [Mesa-dev] [PATCHv2 04/13] glsl: protect anonymous struct id with a mutex

2014-08-19 Thread Chia-I Wu 
On Thu, Aug 14, 2014 at 3:03 AM, Ian Romanick i...@freedesktop.org wrote:
 On 07/09/2014 12:47 AM, Chia-I Wu wrote:
 There may be two contexts compiling shaders at the same time, and we want the
 anonymous struct id to be globally unique.

 Signed-off-by: Chia-I Wu o...@lunarg.com
 ---
  src/glsl/glsl_parser_extras.cpp | 10 --
  1 file changed, 8 insertions(+), 2 deletions(-)

 diff --git a/src/glsl/glsl_parser_extras.cpp 
 b/src/glsl/glsl_parser_extras.cpp
 index b327c2b..ad31469 100644
 --- a/src/glsl/glsl_parser_extras.cpp
 +++ b/src/glsl/glsl_parser_extras.cpp
 @@ -1347,9 +1347,15 @@ ast_struct_specifier::ast_struct_specifier(const char 
 *identifier,
  ast_declarator_list 
 *declarator_list)
  {
 if (identifier == NULL) {
 +  static mtx_t mutex = _MTX_INITIALIZER_NP;
static unsigned anon_count = 1;
 -  identifier = ralloc_asprintf(this, #anon_struct_%04x, anon_count);
 -  anon_count++;
 +  unsigned count;
 +
 +  mtx_lock(mutex);
 +  count = anon_count++;
 +  mtx_unlock(mutex);

 My previous feedback on this was to try and use some sort of atomic
 counter when available.  I'm not excited about the performance hit of
 taking a lock here.  Although, this probably isn't hit too much.  After
 this lands, can you submit an enhancement bug for this issue?
Sure.  I responded to that feedback without getting any ACK or NACK.

 +
 +  identifier = ralloc_asprintf(this, #anon_struct_%04x, count);
 }
 name = identifier;
 this-declarations.push_degenerate_list_at_head(declarator_list-link);





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

Re: [Mesa-dev] [PATCHv2 04/13] glsl: protect anonymous struct id with a mutex

2014-08-13 Thread Ian Romanick 
On 07/09/2014 12:47 AM, Chia-I Wu wrote:
 There may be two contexts compiling shaders at the same time, and we want the
 anonymous struct id to be globally unique.
 
 Signed-off-by: Chia-I Wu o...@lunarg.com
 ---
  src/glsl/glsl_parser_extras.cpp | 10 --
  1 file changed, 8 insertions(+), 2 deletions(-)
 
 diff --git a/src/glsl/glsl_parser_extras.cpp b/src/glsl/glsl_parser_extras.cpp
 index b327c2b..ad31469 100644
 --- a/src/glsl/glsl_parser_extras.cpp
 +++ b/src/glsl/glsl_parser_extras.cpp
 @@ -1347,9 +1347,15 @@ ast_struct_specifier::ast_struct_specifier(const char 
 *identifier,
  ast_declarator_list *declarator_list)
  {
 if (identifier == NULL) {
 +  static mtx_t mutex = _MTX_INITIALIZER_NP;
static unsigned anon_count = 1;
 -  identifier = ralloc_asprintf(this, #anon_struct_%04x, anon_count);
 -  anon_count++;
 +  unsigned count;
 +
 +  mtx_lock(mutex);
 +  count = anon_count++;
 +  mtx_unlock(mutex);

My previous feedback on this was to try and use some sort of atomic
counter when available.  I'm not excited about the performance hit of
taking a lock here.  Although, this probably isn't hit too much.  After
this lands, can you submit an enhancement bug for this issue?

 +
 +  identifier = ralloc_asprintf(this, #anon_struct_%04x, count);
 }
 name = identifier;
 this-declarations.push_degenerate_list_at_head(declarator_list-link);
 

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

Re: [Mesa-dev] [PATCHv2 04/13] glsl: protect anonymous struct id with a mutex

2014-08-13 Thread Kenneth Graunke 
On Wednesday, August 13, 2014 12:03:10 PM Ian Romanick wrote:
 On 07/09/2014 12:47 AM, Chia-I Wu wrote:
  There may be two contexts compiling shaders at the same time, and we want 
  the
  anonymous struct id to be globally unique.
  
  Signed-off-by: Chia-I Wu o...@lunarg.com
  ---
   src/glsl/glsl_parser_extras.cpp | 10 --
   1 file changed, 8 insertions(+), 2 deletions(-)
  
  diff --git a/src/glsl/glsl_parser_extras.cpp 
  b/src/glsl/glsl_parser_extras.cpp
  index b327c2b..ad31469 100644
  --- a/src/glsl/glsl_parser_extras.cpp
  +++ b/src/glsl/glsl_parser_extras.cpp
  @@ -1347,9 +1347,15 @@ ast_struct_specifier::ast_struct_specifier(const 
  char *identifier,
 ast_declarator_list *declarator_list)
   {
  if (identifier == NULL) {
  +  static mtx_t mutex = _MTX_INITIALIZER_NP;
 static unsigned anon_count = 1;
  -  identifier = ralloc_asprintf(this, #anon_struct_%04x, anon_count);
  -  anon_count++;
  +  unsigned count;
  +
  +  mtx_lock(mutex);
  +  count = anon_count++;
  +  mtx_unlock(mutex);
 
 My previous feedback on this was to try and use some sort of atomic
 counter when available.  I'm not excited about the performance hit of
 taking a lock here.  Although, this probably isn't hit too much.  After
 this lands, can you submit an enhancement bug for this issue?

Yeah, I don't think this is hit often at all...

--Ken

signature.asc
Description: This is a digitally signed message part.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev