BerkeleyLab / caffeine

A parallel runtime library for Fortran compilers
https://berkeleylab.github.io/caffeine/
Other
37 stars 7 forks source link

Fix calls in `caffeine.c` #108

Closed ktras closed 5 months ago

ktras commented 5 months ago

Closes #74.

ktras commented 5 months ago

The following two warnings still exist because when attempting to remove the warnings by dereferencing the user_op args to match what the functions seems to expect, then there is a runtime crash during the tests. These function calls involves macro expansions for both the functions and for the argument types, so I may have misread what was needed to fix the call and remove the warning. Or perhaps the call doesn't need fixing.

././src/caffeine/caffeine.c:142:103: warning: passing argument 9 of 'gasnete_tm_reduce_nb' from incompatible pointer type [-Wincompatible-pointer-types]
  142 |       team, result_image-1, a_address, a_address, GEX_DT_USER, c_sizeof_a, num_elements, GEX_OP_USER, user_op, &c_sizeof_a, 0
      |                                                                                                       ^~~~~~~
      |                                                                                                       |
      |                                                                                                       void (**)(const void *, void *, size_t,  const void *) {aka void (**)(const void *, void *, long unsigned int,  const void *)}
/Users/kateras/.local/include/gasnet_coll.h:400:60: note: in definition of macro 'gex_Coll_ReduceToOneNB'
  400 |         gasnete_tm_reduce_nb(tm,root,dst,src,dt,dts,dtc,op,fn,cdata,flags,0 GASNETI_THREAD_GET)
      |                                                            ^~
/Users/kateras/.local/include/gasnet_coll.h:396:58: note: expected 'gex_Coll_ReduceFn_t' {aka 'void (*)(const void *, void *, long unsigned int,  const void *)'} but argument is of type 'void (**)(const void *, void *, size_t,  const void *)' {aka 'void (**)(const void *, void *, long unsigned int,  const void *)'}
  396 |                        gex_OP_t _op, gex_Coll_ReduceFn_t _user_op, void * _user_cdata,
      |                                      ~~~~~~~~~~~~~~~~~~~~^~~~~~~~
././src/caffeine/caffeine.c:146:103: warning: passing argument 8 of 'gasnete_tm_reduce_all_nb' from incompatible pointer type [-Wincompatible-pointer-types]
  146 |       team,                 a_address, a_address, GEX_DT_USER, c_sizeof_a, num_elements, GEX_OP_USER, user_op, &c_sizeof_a, 0
      |                                                                                                       ^~~~~~~
      |                                                                                                       |
      |                                                                                                       void (**)(const void *, void *, size_t,  const void *) {aka void (**)(const void *, void *, long unsigned int,  const void *)}
/Users/kateras/.local/include/gasnet_coll.h:413:59: note: in definition of macro 'gex_Coll_ReduceToAllNB'
  413 |         gasnete_tm_reduce_all_nb(tm,dst,src,dt,dts,dtc,op,fn,cdata,flags,0 GASNETI_THREAD_GET)
      |                                                           ^~
/Users/kateras/.local/include/gasnet_coll.h:409:59: note: expected 'gex_Coll_ReduceFn_t' {aka 'void (*)(const void *, void *, long unsigned int,  const void *)'} but argument is of type 'void (**)(const void *, void *, size_t,  const void *)' {aka 'void (**)(const void *, void *, long unsigned int,  const void *)'}
  409 |                         gex_OP_t _op, gex_Coll_ReduceFn_t _user_op, void * _user_cdata,
      |                                       ~~~~~~~~~~~~~~~~~~~~^~~~~~~~
bonachea commented 5 months ago

The following two warnings still exist because when attempting to remove the warnings by dereferencing the user_op args to match what the functions seems to expect, then there is a runtime crash during the tests.

I suspect you deployed the wrong fix. This is a warning about mismatched types in code that is already believed to operate correctly. In such a situation adding or removing a deference operator to assuage the typechecker is almost never the right fix, because it changes the semantics of the code that already runs correctly (to something that usually does not).

I believe the actual problem here is with the type declarations, specifically the C declaration of the caf_co_reduce function. gex_Coll_ReduceFn_t is already a pointer-to-function type, and thus compatible with the c_funptr provided from the Fortran side of the call. The problem is the extraneous * in the C declaration of the user_op formal argument to caf_co_reduce.