Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

OSHMEM/SHMEM: fix warnings regarding types and unused variables in shmem #12648

Merged
merged 1 commit into from
Jul 11, 2024
Merged

OSHMEM/SHMEM: fix warnings regarding types and unused variables in shmem #12648

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 12 additions & 13 deletions oshmem/mca/spml/spml.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -597,12 +597,11 @@ typedef int (*mca_spml_base_module_test_all_vector_fn_t)(void *ivars,
*
* @return OSHMEM_SUCCESS or failure status.
*/
typedef int (*mca_spml_base_module_test_any_vector_fn_t)(void *ivars,
int cmp,
void *cmp_values,
size_t nelems,
const int *status,
int datatype);
typedef size_t (*mca_spml_base_module_test_any_vector_fn_t)(void *ivars, int cmp,
void *cmp_values,
size_t nelems,
const int *status,
int datatype);

/*
* Indicate whether at least one variable within an array of variables on the local PE meets
Expand All @@ -624,13 +623,13 @@ typedef int (*mca_spml_base_module_test_any_vector_fn_t)(void *ivars,
*
* @return OSHMEM_SUCCESS or failure status.
*/
typedef int (*mca_spml_base_module_test_some_vector_fn_t)(void *ivars,
int cmp,
void *cmp_values,
size_t nelems,
size_t *indices,
const int *status,
int datatype);
typedef size_t (*mca_spml_base_module_test_some_vector_fn_t)(void *ivars,
int cmp,
void *cmp_values,
size_t nelems,
size_t *indices,
const int *status,
int datatype);

/*
* Registers the arrival of a PE at a synchronization point.
Expand Down
39 changes: 20 additions & 19 deletions oshmem/mca/spml/ucx/spml_ucx.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1663,44 +1663,44 @@ int mca_spml_ucx_put_signal_nb(shmem_ctx_t ctx, void* dst_addr, size_t size,
void mca_spml_ucx_wait_until_all(void *ivars, int cmp, void
*cmp_value, size_t nelems, const int *status, int datatype)
{
return ;
RUNTIME_SHMEM_NOT_IMPLEMENTED_API_ABORT();
}

/* This routine is not implemented */
size_t mca_spml_ucx_wait_until_any(void *ivars, int cmp, void
*cmp_value, size_t nelems, const int *status, int datatype)
{
return OSHMEM_ERR_NOT_IMPLEMENTED;
RUNTIME_SHMEM_NOT_IMPLEMENTED_API_ABORT_RET_SIZE_T();
}

/* This routine is not implemented */
size_t mca_spml_ucx_wait_until_some(void *ivars, int cmp, void
*cmp_value, size_t nelems, size_t *indices, const int *status, int
datatype)
{
return OSHMEM_ERR_NOT_IMPLEMENTED;
RUNTIME_SHMEM_NOT_IMPLEMENTED_API_ABORT_RET_SIZE_T();
}

/* This routine is not implemented */
void mca_spml_ucx_wait_until_all_vector(void *ivars, int cmp, void
*cmp_values, size_t nelems, const int *status, int datatype)
{
return ;
RUNTIME_SHMEM_NOT_IMPLEMENTED_API_ABORT();
}

/* This routine is not implemented */
size_t mca_spml_ucx_wait_until_any_vector(void *ivars, int cmp, void
*cmp_value, size_t nelems, const int *status, int datatype)
{
return OSHMEM_ERR_NOT_IMPLEMENTED;
RUNTIME_SHMEM_NOT_IMPLEMENTED_API_ABORT_RET_SIZE_T();
}

/* This routine is not implemented */
size_t mca_spml_ucx_wait_until_some_vector(void *ivars, int cmp, void
*cmp_value, size_t nelems, size_t *indices, const int *status, int
datatype)
{
return OSHMEM_ERR_NOT_IMPLEMENTED;
RUNTIME_SHMEM_NOT_IMPLEMENTED_API_ABORT_RET_SIZE_T();
}

/* This routine is not implemented */
Expand All @@ -1714,36 +1714,39 @@ int mca_spml_ucx_test_all(void *ivars, int cmp, void *cmp_value,
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;
RUNTIME_SHMEM_NOT_IMPLEMENTED_API_ABORT_RET_SIZE_T();
}

/* This routine is not implemented */
size_t mca_spml_ucx_test_some(void *ivars, int cmp, void *cmp_value,
size_t nelems, size_t *indices, const int *status, int datatype)
{
return OSHMEM_ERR_NOT_IMPLEMENTED;
RUNTIME_SHMEM_NOT_IMPLEMENTED_API_ABORT_RET_SIZE_T();
}

/* This routine is not implemented */
int mca_spml_ucx_test_all_vector(void *ivars, int cmp, void
*cmp_values, size_t nelems, const int *status, int datatype)
int mca_spml_ucx_test_all_vector(void *ivars, int cmp, void *cmp_values,
size_t nelems, const int *status,
int datatype)
{
return OSHMEM_ERR_NOT_IMPLEMENTED;
Copy link
Contributor

@MamziB MamziB Jul 3, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@roiedanino can you please explain how this line leads to a warning? I understand the warning for size_t but not for int. Same for void.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one does not, but it's incorrect to return a return code as it doesn't match specifications to do so, the caller won't expect a return code and won't interpret it as such

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could internally return ssize_t to be able to encode errors. That would resolve the warning.

Copy link
Contributor Author

@roiedanino roiedanino Jul 4, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So we will need to add int* return_value parameter to all of those functions to be able to return both rc and the actual value needed to be returned to the caller by specifications

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@devreal @MamziB Wdyt?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought these functions return size_t in the standard? If you replace size_t with ssize_t (signed size_t) you can return a negative error code and the index/size required by the standard. That will silence the compiler warnings I pointed out (which complained about an unsigned value being tested for negative values)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So we will need to add int* return_value parameter to all of those functions to be able to return both rc and the actual value needed to be returned to the caller by specifications

I thought these functions return size_t in the standard? If you replace size_t with ssize_t (signed size_t) you can return a negative error code and the index/size required by the standard. That will silence the compiler warnings I pointed out (which complained about an unsigned value being tested for negative values)

@roiedanino I would suggest keeping the changes to a minimum for now, we can think about dealing with error codes for functions that do not return integers once we have the implementation in. I suggest printing an error/warning and breaking or returning SIZE_MAX for now.

@devreal Yes, standard mandates that this function return size_t and I think we'd better not to change the return code.

}

/* This routine is not implemented */
int mca_spml_ucx_test_any_vector(void *ivars, int cmp, void
*cmp_values, size_t nelems, const int *status, int datatype)
size_t mca_spml_ucx_test_any_vector(void *ivars, int cmp,
void *cmp_values, size_t nelems,
const int *status, int datatype)
{
return OSHMEM_ERR_NOT_IMPLEMENTED;
RUNTIME_SHMEM_NOT_IMPLEMENTED_API_ABORT_RET_SIZE_T();
}

/* This routine is not implemented */
int mca_spml_ucx_test_some_vector(void *ivars, int cmp, void
*cmp_values, size_t nelems, size_t *indices, const int *status, int
datatype)
size_t mca_spml_ucx_test_some_vector(void *ivars, int cmp,
void *cmp_values, size_t nelems,
size_t *indices, const int *status,
int datatype)
{
return OSHMEM_ERR_NOT_IMPLEMENTED;
RUNTIME_SHMEM_NOT_IMPLEMENTED_API_ABORT_RET_SIZE_T();
}

/* This routine is not implemented */
Expand Down Expand Up @@ -1855,5 +1858,3 @@ int mca_spml_ucx_team_reduce(shmem_team_t team, void
{
return OSHMEM_ERR_NOT_IMPLEMENTED;
}


9 changes: 4 additions & 5 deletions oshmem/mca/spml/ucx/spml_ucx.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -259,11 +259,10 @@ extern size_t mca_spml_ucx_test_some(void *ivars, int cmp, void *cmp_value,
size_t nelems, size_t *indices, const int *status, int datatype);
extern int mca_spml_ucx_test_all_vector(void *ivars, int cmp, void
*cmp_values, size_t nelems, const int *status, int datatype);
extern int mca_spml_ucx_test_any_vector(void *ivars, int cmp, void
*cmp_values, size_t nelems, const int *status, int datatype);
extern int mca_spml_ucx_test_some_vector(void *ivars, int cmp, void
*cmp_values, size_t nelems, size_t *indices, const int *status, int
datatype);
extern size_t mca_spml_ucx_test_any_vector(void *ivars, int cmp, void *cmp_values, size_t nelems,
const int *status, int datatype);
extern size_t mca_spml_ucx_test_some_vector(void *ivars, int cmp, void *cmp_values, size_t nelems,
size_t *indices, const int *status, int datatype);
extern int mca_spml_ucx_team_sync(shmem_team_t team);
extern int mca_spml_ucx_team_my_pe(shmem_team_t team);
extern int mca_spml_ucx_team_n_pes(shmem_team_t team);
Expand Down
1 change: 0 additions & 1 deletion oshmem/mca/spml/ucx/spml_ucx_component.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -463,7 +463,6 @@ static int mca_spml_ucx_component_fini(void)
volatile int fenced = 0;
int i;
int ret = OSHMEM_SUCCESS;
mca_spml_ucx_ctx_t *ctx;
gleon99 marked this conversation as resolved.
Show resolved Hide resolved

opal_progress_unregister(spml_ucx_default_progress);
if (mca_spml_ucx.active_array.ctxs_count) {
Expand Down
5 changes: 2 additions & 3 deletions oshmem/mca/sshmem/ucx/sshmem_ucx_module.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -181,12 +181,11 @@ segment_create(map_segment_t *ds_buf,
{
mca_spml_ucx_t *spml = (mca_spml_ucx_t*)mca_spml.self;
unsigned flags = UCP_MEM_MAP_ALLOCATE;
int status;

if (hint & SHMEM_HINT_DEVICE_NIC_MEM) {
#if HAVE_DECL_UCS_MEMORY_TYPE_RDMA
status = segment_create_internal(ds_buf, NULL, size, flags,
UCS_MEMORY_TYPE_RDMA, 3);
int status = segment_create_internal(ds_buf, NULL, size,
flags, UCS_MEMORY_TYPE_RDMA, 3);
if (status == OSHMEM_SUCCESS) {
ds_buf->alloc_hints = hint;
ds_buf->allocator = &sshmem_ucx_allocator;
Expand Down
12 changes: 12 additions & 0 deletions oshmem/runtime/runtime.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -237,6 +237,18 @@ OSHMEM_DECLSPEC int oshmem_shmem_register_params(void);

#endif /* OSHMEM_PARAM_CHECK */

#define RUNTIME_SHMEM_NOT_IMPLEMENTED_API_ABORT() \
do { \
SHMEM_API_ERROR("Called non-implemented API: %s", __func__); \
oshmem_shmem_abort(OSHMEM_ERR_NOT_IMPLEMENTED); \
} while (0)

#define RUNTIME_SHMEM_NOT_IMPLEMENTED_API_ABORT_RET_SIZE_T() \
do { \
RUNTIME_SHMEM_NOT_IMPLEMENTED_API_ABORT(); \
return SIZE_MAX; \
} while (0)

END_C_DECLS

#endif /* OSHMEM_SHMEM_RUNTIME_H */
43 changes: 11 additions & 32 deletions oshmem/shmem/c/shmem_test_ivars.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@
#define SHMEM_TYPE_TEST_ALL(type_name, type, code, prefix) \
int prefix##type_name##_test_all(volatile type *ivars, size_t nelems, const int *status, int cmp, type value) \
{ \
int rc = OSHMEM_SUCCESS; \
int rc; \
\
RUNTIME_CHECK_INIT(); \
\
Expand All @@ -138,81 +138,60 @@
return rc; \
}


#define SHMEM_TYPE_TEST_ANY(type_name, type, code, prefix) \
size_t prefix##type_name##_test_any(volatile type *ivars, size_t nelems, const int *status, int cmp, type value) \
{ \
size_t rc = 0; \
\
RUNTIME_CHECK_INIT(); \
\
rc = MCA_SPML_CALL(test_any( \
(void*)ivars, \
cmp, \
(void*)&value, \
nelems, status, code)); \
RUNTIME_CHECK_IMPL_RC(rc); \
\
return rc; \
return MCA_SPML_CALL(test_any( \
(void*)ivars, \
cmp, \
(void*)&value, \
nelems, status, code)); \
}


#define SHMEM_TYPE_TEST_SOME(type_name, type, code, prefix) \
size_t prefix##type_name##_test_some(volatile type *ivars, size_t nelems, size_t *indices, const int *status, int cmp, type value) \
{ \
size_t rc = 0; \
\
RUNTIME_CHECK_INIT(); \
\
rc = MCA_SPML_CALL(test_some( \
return MCA_SPML_CALL(test_some( \
(void*)ivars, \
cmp, \
(void*)&value, \
nelems, indices, status, code)); \
RUNTIME_CHECK_IMPL_RC(rc); \
\
return rc; \
}

#define SHMEM_TYPE_TEST_ANY_VECTOR(type_name, type, code, prefix) \
size_t prefix##type_name##_test_any_vector(volatile type *ivars, size_t nelems, const int *status, int cmp, type *values) \
{ \
size_t rc = 0; \
\
RUNTIME_CHECK_INIT(); \
\
rc = MCA_SPML_CALL(test_any_vector( \
return MCA_SPML_CALL(test_any_vector( \
(void*)ivars, \
cmp, \
(void*)values, \
nelems, status, code)); \
RUNTIME_CHECK_IMPL_RC(rc); \
\
return rc; \
}

#define SHMEM_TYPE_TEST_SOME_VECTOR(type_name, type, code, prefix) \
size_t prefix##type_name##_test_some_vector(volatile type *ivars, size_t nelems, size_t *indices, const int *status, int cmp, type *values) \
{ \
size_t rc = 0; \
\
RUNTIME_CHECK_INIT(); \
\
rc = MCA_SPML_CALL(test_some_vector( \
return MCA_SPML_CALL(test_some_vector( \
(void*)ivars, \
cmp, \
(void*)values, \
nelems, indices, status, code)); \
RUNTIME_CHECK_IMPL_RC(rc); \
\
return rc; \
}


#define SHMEM_TYPE_TEST_ALL_VECTOR(type_name, type, code, prefix) \
int prefix##type_name##_test_all_vector(volatile type *ivars, size_t nelems, const int *status, int cmp, type *values) \
int prefix##type_name##_test_all_vector(volatile type *ivars, size_t nelems, const int *status, int cmp, type *values) \
{ \
int rc = OSHMEM_SUCCESS; \
int rc; \
\
RUNTIME_CHECK_INIT(); \
\
Expand Down
Loading
Loading