gmegan / specification

OpenSHMEM Application Programming Interface
http://www.openshmem.org
1 stars 0 forks source link

Remove C11 shmem_sync(team) #99

Closed gmegan closed 5 years ago

gmegan commented 5 years ago

Right now the teams version of shmem_sync has the following functions defined:

C11:

int shmem_sync(shmem_team_t team);

C

int shmem_team_sync(shmem_team_t team);
void shmem_sync(int PE_start, int logPE_stride, int PE_size, long *pSync);

So there is a name conflict with shmem_sync C function vs C11 generic.

I am able to get a stub program to compile and run that supports this, but I worry that it causes problems both with portability and profiling.

The following works because the macro def for shmem_sync comes after the function def and overrides it, and the preprocessor only makes a single pass. So compiler encounters shmem_sync in main as a macro, resolves it to shmem_sync in preprocess, which then resolves to the function version of shmem_sync for the active set. I don't get errors or warning from the listed code in clang.

I have a strange problem in that I really like generics and don't seem to be bothered by 100 line compiler errors, so I have no perspective here. I would do this all day, but I think this is probably not the popular opinion.

Also that ## _VAARGS isn't 100% portable either, so a fully portable implementation would need that macro counting trick to do this.

On the other hand, if the goal is to remove shmem_sync(active set), then having the generic would let programs start using shmem_sync(team) instead of shmem_team_sync. Then when the active set version goes away, the teams version could just become the C/C++ version.

#include <stdio.h>

typedef struct {int _;} shmem_team_s;
typedef shmem_team_s *shmem_team_t;

int shmem_team_sync(shmem_team_t team) {
  printf ("sync team: %p\n", team);
  return 1;
}

void shmem_sync(int PE_start, int logPE_stride, int PE_size, long *pSync)
{
  printf ("sync active set: %d, %d, %d, %p\n", PE_start, logPE_stride, PE_size, pSync);
}

#define shmem_sync(T, ...)              \
  _Generic( (T),                    \
        shmem_team_t: shmem_team_sync,      \
        int:          shmem_sync)( T, ##__VA_ARGS__ )

int main()
{
  shmem_team_t team = NULL;
  shmem_team_sync(team);
  shmem_team_sync(NULL);
  shmem_sync(1, 2, 3, NULL);
}
nspark commented 5 years ago

Maybe I too am the wrong person to ask, but I don't see the problem here. We'll be providing two C89-compatible functions and one C11 type-generic interface:

// C89
int shmem_team_sync(shmem_team_t team);
void shmem_sync(int PE_start, int logPE_stride, int PE_size, long *pSync);
// C11 only
int shmem_sync(shmem_team_t team);

As you say, ##__VA_ARGS__ is not strictly portable, but other macro techniques—which we've already had to use elsewhere in the API for C11 generics—can make this 100% portable for C11-compatible compilers.

gmegan commented 5 years ago

This is good with me. I wanted to check on this after some discussion about the other collective routines seemed to indicate that there was a problem in having C11 functions with name overlap onto C89 functions. Maybe this was a misunderstanding, since this seems to be fine.