Opened 7 years ago

Last modified 4 years ago

#1058 new bug

Re-consider the thread tls interfaces

Reported by: jayesh Owned by:
Priority: major Milestone: future
Component: mpich Keywords:
Cc:

Description (last modified by balaji)

We need to re-consider the interfaces for MPID_Thread_tls*/MPIU_Thread_tls* functions. For example the device tls get interface is currently,

void MPID_Thread_tls_get(MPID_Thread_tls_t * tls, void value);

This makes sense since since OS tls get interfaces typically return a (void *).
However, there are instances of code in MPICH2 that use the above interfaces with a non-void value,

eg:
=========== src/util/dbg/dbg_printf.c =============
FILE *fp;
MPID_Thread_tls_get(&dbg_tls_key, &fp);
=========== src/util/dbg/dbg_printf.c =============

Since there is no explicit type cast for the value (2nd arg to tls_get) the code above fails to compile on atleast the Visual Studio compiler (The current code base now compiles on VS because the interfaces are macros and there is an explicit type cast before calling the windows tls functions).

An obvious explicit type cast for the second arg to (void ) results in "type-punned" warnings from compilers like gcc.

-Jayesh

Change History (4)

comment:1 Changed 7 years ago by jayesh

We need to re-consider the interfaces for MPID_Thread_tls*/MPIU_Thread_tls* functions. For example the device tls get interface is currently,

void MPID_Thread_tls_get(MPID_Thread_tls_t * tls, void ** value);

This makes sense since since OS tls get interfaces typically return a (void *). However, there are instances of code in MPICH2 that use the above interfaces with a non-void value,

eg:

=========== src/util/dbg/dbg_printf.c ============= 

FILE *fp; 
MPID_Thread_tls_get(&dbg_tls_key, &fp); 

=========== src/util/dbg/dbg_printf.c =============

Since there is no explicit type cast for the value (2nd arg to tls_get) the code above fails to compile on atleast the Visual Studio compiler (The current code base now compiles on VS because the interfaces are macros and there is an explicit type cast before calling the windows tls functions).

An obvious explicit type cast for the second arg to (void ) results in "type-punned" warnings from compilers like gcc.

-Jayesh

comment:2 Changed 7 years ago by buntinas

If windows doesn't give the type pun warning, then can we add a wrapper macro for windows that adds the cast before calling the function?

Something like:

#ifdef WINDOWS
#define MPID_Thread_tls_get(key, val) MPIU_Thread_tls_get(key, (void **)val)
#else
#define MPID_Thread_tls_get(key, val) MPIU_Thread_tls_get(key, val)
#endif

Of course it might be cleaner to do this with MPIU_Thread_tls_get(), since that already has a different implementation for windows, posix, solaris, etc.

comment:3 Changed 7 years ago by jayesh

I think the compilers on unix (eg: gcc) don't complain (incompatible arg warnings) because the TLS functions are implemented as macros in unix.

-Jayesh

comment:4 Changed 4 years ago by balaji

  • Description modified (diff)
  • Owner jayesh deleted
Note: See TracTickets for help on using tickets.