[Mesa-dev] [PATCH] glsl: add a missing call to _mesa_locale_init

2015-07-02 Thread Erik Faye-Lund 
After c61bc6e (util: port _mesa_strto[df] to C), make check
fails due to a missing _mesa_locale_init. Fixup this oversight.

Signed-off-by: Erik Faye-Lund kusmab...@gmail.com
Tested-by: Vinson Lee v...@freedesktop.org
---
 src/glsl/test.cpp | 4 
 1 file changed, 4 insertions(+)

diff --git a/src/glsl/test.cpp b/src/glsl/test.cpp
index b1ff92e..9dc7c47 100644
--- a/src/glsl/test.cpp
+++ b/src/glsl/test.cpp
@@ -37,6 +37,7 @@
 #include stdio.h
 #include stdlib.h
 #include string.h
+#include util/strtod.h
 
 #include test_optpass.h
 
@@ -67,6 +68,9 @@ static const char *extract_command_from_argv(int *argc, char 
**argv)
 int main(int argc, char **argv)
 {
const char *command = extract_command_from_argv(argc, argv);
+
+   _mesa_locale_init();
+
if (strcmp(command, optpass) == 0) {
   return test_optpass(argc, argv);
} else {
-- 
2.1.4

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

Re: [Mesa-dev] [PATCH] glsl: add a missing call to _mesa_locale_init

2015-07-02 Thread Ilia Mirkin 
Can this be done at dlopen/init time? For example what happens if you do

static int foo = _mesa_locale_init()

IIRC things like that are possible in C++, not sure about C.

On Thu, Jul 2, 2015 at 5:18 PM, Erik Faye-Lund kusmab...@gmail.com wrote:
 After c61bc6e (util: port _mesa_strto[df] to C), make check
 fails due to a missing _mesa_locale_init. Fixup this oversight.

 Signed-off-by: Erik Faye-Lund kusmab...@gmail.com
 Tested-by: Vinson Lee v...@freedesktop.org
 ---
  src/glsl/test.cpp | 4 
  1 file changed, 4 insertions(+)

 diff --git a/src/glsl/test.cpp b/src/glsl/test.cpp
 index b1ff92e..9dc7c47 100644
 --- a/src/glsl/test.cpp
 +++ b/src/glsl/test.cpp
 @@ -37,6 +37,7 @@
  #include stdio.h
  #include stdlib.h
  #include string.h
 +#include util/strtod.h

  #include test_optpass.h

 @@ -67,6 +68,9 @@ static const char *extract_command_from_argv(int *argc, 
 char **argv)
  int main(int argc, char **argv)
  {
 const char *command = extract_command_from_argv(argc, argv);
 +
 +   _mesa_locale_init();
 +
 if (strcmp(command, optpass) == 0) {
return test_optpass(argc, argv);
 } else {
 --
 2.1.4

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

Re: [Mesa-dev] [PATCH] glsl: add a missing call to _mesa_locale_init

2015-07-02 Thread Matt Turner 
On Thu, Jul 2, 2015 at 2:18 PM, Erik Faye-Lund kusmab...@gmail.com wrote:
 After c61bc6e (util: port _mesa_strto[df] to C), make check
 fails due to a missing _mesa_locale_init. Fixup this oversight.

 Signed-off-by: Erik Faye-Lund kusmab...@gmail.com
 Tested-by: Vinson Lee v...@freedesktop.org

Reviewed-by: Matt Turner matts...@gmail.com

I'll commit this soon.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] glsl: add a missing call to _mesa_locale_init

2015-07-02 Thread Ilia Mirkin 
On Thu, Jul 2, 2015 at 5:54 PM, Matt Turner matts...@gmail.com wrote:
 On Thu, Jul 2, 2015 at 2:22 PM, Ilia Mirkin imir...@alum.mit.edu wrote:
 Can this be done at dlopen/init time? For example what happens if you do

 static int foo = _mesa_locale_init()

 IIRC things like that are possible in C++, not sure about C.

 gcc has __attribute__((constructor)).

 But I don't think we really care... Erik's series converted the strtod
 code from C++ to C (including moving locale init from being a static
 constructor to being called in one_time_init) and fixing the memory
 leak.

Well, this is just going to happen over and over again, I was hoping
there was an easy way to do static initializers in C. If not, then I
guess we're stuck with this.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] glsl: add a missing call to _mesa_locale_init

2015-07-02 Thread Matt Turner 
On Thu, Jul 2, 2015 at 2:22 PM, Ilia Mirkin imir...@alum.mit.edu wrote:
 Can this be done at dlopen/init time? For example what happens if you do

 static int foo = _mesa_locale_init()

 IIRC things like that are possible in C++, not sure about C.

gcc has __attribute__((constructor)).

But I don't think we really care... Erik's series converted the strtod
code from C++ to C (including moving locale init from being a static
constructor to being called in one_time_init) and fixing the memory
leak.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] glsl: add a missing call to _mesa_locale_init

2015-07-02 Thread Erik Faye-Lund 
On Thu, Jul 2, 2015 at 2:56 PM, Ilia Mirkin imir...@alum.mit.edu wrote:
 On Thu, Jul 2, 2015 at 5:54 PM, Matt Turner matts...@gmail.com wrote:
 On Thu, Jul 2, 2015 at 2:22 PM, Ilia Mirkin imir...@alum.mit.edu wrote:
 Can this be done at dlopen/init time? For example what happens if you do

 static int foo = _mesa_locale_init()

 IIRC things like that are possible in C++, not sure about C.

 gcc has __attribute__((constructor)).

 But I don't think we really care... Erik's series converted the strtod
 code from C++ to C (including moving locale init from being a static
 constructor to being called in one_time_init) and fixing the memory
 leak.

 Well, this is just going to happen over and over again, I was hoping
 there was an easy way to do static initializers in C. If not, then I
 guess we're stuck with this.

The good news is that the breakage was noticed real quick. I agree
that it'd be awesome to have this happen automatically, but AFAIK
there's no perfect solution for this:

* C++ static object initializer leads to libc++ dependencies.
* __attribute__((constructor)) is compiler specific.
* Naively mutex-protecting initialization leads to overhead in the common case.
* Lighter-weight double checked locks are tricky to implement, and
  also have some overhead
* pthread_once() is not available on Windows, and have some overhead

Generally, it leaves a bad taste in the mouth to have to know about
compiler-internals for being able to use the compiler. This could be
slightly mitigated by introducing something like _mesa_glsl_init(),
which means that call-sites only need to know about initializing the
compiler.

However, such a function would only call _mesa_locale_init() at this
point, so we need to know the same amount of things. I'd introduce
_mesa_glsl_init() only if it turns out we need to do more. Besides,
new call-sites for _mesa_strto{d,f} will crash visible and hard if
introduced without _mesa_locale_init.

So, while not being awesome, I think this is the lesser evil for now.

But maybe it would be more palatable to move this into
initialize_context_to_defaults() instead, which is a shared
initializer for stand-alone compilers? Perhaps it's more likely that
this function gets called by other non-driver call-sites... It would
also reduce the number of call-sites by one.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] glsl: add a missing call to _mesa_locale_init

2015-07-02 Thread Ilia Mirkin 
On Thu, Jul 2, 2015 at 6:23 PM, Erik Faye-Lund kusmab...@gmail.com wrote:
 On Thu, Jul 2, 2015 at 2:56 PM, Ilia Mirkin imir...@alum.mit.edu wrote:
 On Thu, Jul 2, 2015 at 5:54 PM, Matt Turner matts...@gmail.com wrote:
 On Thu, Jul 2, 2015 at 2:22 PM, Ilia Mirkin imir...@alum.mit.edu wrote:
 Can this be done at dlopen/init time? For example what happens if you do

 static int foo = _mesa_locale_init()

 IIRC things like that are possible in C++, not sure about C.

 gcc has __attribute__((constructor)).

 But I don't think we really care... Erik's series converted the strtod
 code from C++ to C (including moving locale init from being a static
 constructor to being called in one_time_init) and fixing the memory
 leak.

 Well, this is just going to happen over and over again, I was hoping
 there was an easy way to do static initializers in C. If not, then I
 guess we're stuck with this.

 The good news is that the breakage was noticed real quick. I agree
 that it'd be awesome to have this happen automatically, but AFAIK
 there's no perfect solution for this:

 * C++ static object initializer leads to libc++ dependencies.
 * __attribute__((constructor)) is compiler specific.
 * Naively mutex-protecting initialization leads to overhead in the common 
 case.
 * Lighter-weight double checked locks are tricky to implement, and
   also have some overhead
 * pthread_once() is not available on Windows, and have some overhead

Sounds good. Just wanted to make sure some alternatives were
considered before peppering main functions with _mesa_locale_init.

Cheers,

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