[mpich2-dev] Apparent bypass of correct macros in collective operation code

[Dave Goodell]

Good catch, Joe.  These ought to be changed to use the  
MPIU_THREADPRIV_* macros.  I'll forward this to mpich2-maint@ to make  
sure we don't forget to fix it.

-Dave

On Jul 10, 2008, at 8:47 PM, Joe Ratterman wrote:

> I was recently doing some thread hacking, and I found that some of  
> my changes where causing a problem in the collective operation C  
> files: mpich2/src/mpi/coll/op*.c
>
> Specifically, this sort of code was a problem since I got rid of  
> the op_errno field in the MPICH_PerThread object.
> https://svn.mcs.anl.gov/repos/mpi/mpich2/trunk/src/mpi/coll/opbor.c
>    165      default: {
>    166          MPICH_PerThread_t *p;
>    167          MPIR_GetPerThread(&p);
>    168          p->op_errno = MPIR_Err_create_code( MPI_SUCCESS,  
> MPIR_ERR_RECOVERABLE, FCNAME, __LINE__, MPI_ERR_OP,  
> "**opundefined","**opundefined %s", "MPI_BOR" );
>    169          break;
>    170      }
>
> I think that there are macros to do this, as seen in the  
> allreduce.c file (extra lines deleted):
>    117      MPIU_THREADPRIV_DECL;
>    126      MPIU_THREADPRIV_GET;
>    158          MPIU_THREADPRIV_FIELD(op_errno) = 0;
>    473          if (MPIU_THREADPRIV_FIELD(op_errno))
>    474              mpi_errno = MPIU_THREADPRIV_FIELD(op_errno);
>
> With the default macros, that basically does the same thing, but I  
> didn't have to change the .c file--only the header files.  The same  
> thing happens in errutil.c
> https://svn.mcs.anl.gov/repos/mpi/mpich2/trunk/src/mpi/errhan/ 
> errutil.c
>    156  /* These routines export the nest increment and decrement  
> for use in ROMIO */
>    157  void MPIR_Nest_incr_export( void )
>    158  {
>    159      MPICH_PerThread_t *p;
>    160      MPIR_GetPerThread(&p);
>    161      p->nest_count++;
>    162  }
>    163  void MPIR_Nest_decr_export( void )
>    164  {
>    165      MPICH_PerThread_t *p;
>    166      MPIR_GetPerThread(&p);
>    167      p->nest_count--;
>    168  }
>
>
>
> I really think that these places should be using the existing  
> macros to handle the work.
>
>
> Comments?
> Joe Ratterman
> jratt at us.ibm.com