open-mpi / ompi

Open MPI main development repository
https://www.open-mpi.org
Other
2.07k stars 844 forks source link

Warnings in oshmem #12636

Open devreal opened 2 weeks ago

devreal commented 2 weeks ago

Current git main shows a bunch of warnings in oshmem, coming from the the RUNTIME_CHECK_IMPL_RC macro:

../../../../oshmem/runtime/runtime.h:168:11: warning: comparison of integer expressions of different signedness: ‘size_t’ {aka ‘long unsigned int’} and ‘int’ [-Wsign-compare]
  168 |     if (x <= -1)                                                            \
      |           ^~
../../../../oshmem/shmem/c/shmem_test_ivars.c:206:9: note: in expansion of macro ‘RUNTIME_CHECK_IMPL_RC’
  206 |         RUNTIME_CHECK_IMPL_RC(rc);                                  \
      |         ^~~~~~~~~~~~~~~~~~~~~
../../../../oshmem/shmem/c/shmem_test_ivars.c:320:1: note: in expansion of macro ‘SHMEM_TYPE_TEST_SOME_VECTOR’
  320 | SHMEM_TYPE_TEST_SOME_VECTOR(_int32,  int32_t, SHMEM_INT32_T, shmem)
      | ^~~~~~~~~~~~~~~~~~~~~~~~~~~

For x as size_t this check is dropped by the compiler, since x can never be smaller than -1 (size_t being unsigned).

There are functions that return size_t but the returned error value is an error (a negative value), e.g.,

size_t mca_spml_ucx_test_any(void *ivars, int cmp, void *cmp_value,
        size_t nelems, const int *status, int datatype)
{
    return OSHMEM_ERR_NOT_IMPLEMENTED;
}

Not sure how that is supposed to work...

roiedanino commented 5 days ago

Will look into it (can't assign myself for some reason)

roiedanino commented 4 days ago

@MamziB Any reason those functions should return size_t instead of int?

Otherwise, I will change those to return int to match other places where rc is returned.