This is an automated email from the git hooks/post-receive script. It was generated because a ref change was pushed to the repository containing the project "MPICH primary repository". The branch, master has been updated via 5014a82c26f3a01aa0bb190510413ebb8df81001 (commit) via dc9275be03350854fd2dde2012bc48d09a10fc14 (commit) from b9531d3de2cdd2f186dfdb0774db94fe0ebc7b7c (commit) Those revisions listed above that are new to this repository have not appeared on any other notification email; so we list those revisions in full, below. - Log ----------------------------------------------------------------- http://git.mpich.org/mpich.git/commitdiff/5014a82c26f3a01aa0bb190510413ebb8d... commit 5014a82c26f3a01aa0bb190510413ebb8df81001 Author: Xin Zhao <[email protected]> Date: Fri Nov 15 14:01:00 2013 -0600 Add more tests in test/mpi/rma/rmazero.c. Add tests of using derived datatype in MPI_ACCUMULATE to test handling of zero-byte transfer, because packet handler of MPI_ACCUMULATE is differernt for predefined datatype and derived datatype. Signed-off-by: Wesley Bland <[email protected]> diff --git a/test/mpi/rma/rmazero.c b/test/mpi/rma/rmazero.c index 0ea28d7..d3a559a 100644 --- a/test/mpi/rma/rmazero.c +++ b/test/mpi/rma/rmazero.c @@ -105,6 +105,7 @@ int main( int argc, char *argv[] ) MPI_Comm comm; MPI_Win win; MPI_Request req; + MPI_Datatype derived_dtp; MTest_Init( &argc, &argv ); @@ -127,6 +128,9 @@ int main( int argc, char *argv[] ) MPI_Abort( MPI_COMM_WORLD, 1 ); } + MPI_Type_contiguous(2, MPI_INT, &derived_dtp); + MPI_Type_commit(&derived_dtp); + /* The following loop is used to run through a series of communicators * that are subsets of MPI_COMM_WORLD, of size 1 or greater. */ while (MTestGetIntracommGeneral( &comm, 1, 1 )) { @@ -137,7 +141,7 @@ int main( int argc, char *argv[] ) MPI_Comm_rank( comm, &rank ); MPI_Comm_size( comm, &size ); - MPI_Win_create( buf, bufsize, sizeof(int), MPI_INFO_NULL, comm, &win ); + MPI_Win_create( buf, bufsize, 2*sizeof(int), MPI_INFO_NULL, comm, &win ); /* To improve reporting of problems about operations, we change the error handler to errors return */ MPI_Win_set_errhandler( win, MPI_ERRORS_RETURN ); @@ -158,6 +162,10 @@ int main( int argc, char *argv[] ) MPI_Accumulate( rmabuf, count, MPI_INT, TARGET, 0, count, MPI_INT, MPI_SUM, win ); ); + TEST_FENCE_OP("Accumulate_derived", + MPI_Accumulate( rmabuf, count, derived_dtp, TARGET, + 0, count, derived_dtp, MPI_SUM, win ); + ); TEST_FENCE_OP("Get accumulate", MPI_Get_accumulate( rmabuf, count, MPI_INT, result, count, MPI_INT, TARGET, 0, @@ -179,6 +187,10 @@ int main( int argc, char *argv[] ) MPI_Accumulate( rmabuf, count, MPI_INT, TARGET, 0, count, MPI_INT, MPI_SUM, win ); ); + TEST_PT_OP("Accumulate_derived", + MPI_Accumulate( rmabuf, count, derived_dtp, TARGET, 0, + count, derived_dtp, MPI_SUM, win ); + ); TEST_PT_OP("Get accumulate", MPI_Get_accumulate( rmabuf, count, MPI_INT, result, count, MPI_INT, TARGET, 0, count, @@ -201,6 +213,10 @@ int main( int argc, char *argv[] ) MPI_Raccumulate( rmabuf, count, MPI_INT, TARGET, 0, count, MPI_INT, MPI_SUM, win, &req ); ); + TEST_REQ_OP("Raccumulate_derived", req, + MPI_Raccumulate( rmabuf, count, derived_dtp, TARGET, 0, + count, derived_dtp, MPI_SUM, win, &req ); + ); TEST_REQ_OP("Rget_accumulate", req, MPI_Rget_accumulate( rmabuf, count, MPI_INT, result, count, MPI_INT, TARGET, 0, @@ -211,6 +227,8 @@ int main( int argc, char *argv[] ) MTestFreeComm(&comm); } + MPI_Type_free(&derived_dtp); + free( result ); free( buf ); free( rmabuf ); http://git.mpich.org/mpich.git/commitdiff/dc9275be03350854fd2dde2012bc48d09a... commit dc9275be03350854fd2dde2012bc48d09a10fc14 Author: Xin Zhao <[email protected]> Date: Fri Nov 15 13:38:38 2013 -0600 Fix #1701 - cleanup code for zero-size data transfer. Delete code for zero-size data transfer in packet handlers of Put/Accumulate/Accumulate_Immed/Get_AccumulateResp/GetResp/ LockPutUnlock/LockAccumUnlock, because they are redundant. (Note that packet handlers of LockPutUnlock and LockAccumUnlock are for single operation optimization in passive RMA) Zero-size data transfer has already been handled when issuing RMA operations (L146, L258, L369 in src/mpid/ch3/src/ch3u_rma_ops.c and L50 in src/mpid/ch3/src/ch3u_rma_acc_ops.c). RMA operation routines will directly exit if data size is zero. Signed-off-by: Wesley Bland <[email protected]> diff --git a/src/mpid/ch3/src/ch3u_rma_sync.c b/src/mpid/ch3/src/ch3u_rma_sync.c index 7a2553a..732b0da 100644 --- a/src/mpid/ch3/src/ch3u_rma_sync.c +++ b/src/mpid/ch3/src/ch3u_rma_sync.c @@ -3398,13 +3398,6 @@ int MPIDI_CH3_PktHandler_Put( MPIDI_VC_t *vc, MPIDI_CH3_Pkt_t *pkt, type_size); req->dev.recv_data_sz = type_size * put_pkt->count; - if (req->dev.recv_data_sz == 0) { - MPIDI_CH3U_Request_complete( req ); - *buflen = sizeof(MPIDI_CH3_Pkt_t); - *rreqp = NULL; - goto fn_exit; - } - mpi_errno = MPIDI_CH3U_Receive_data_found(req, data_buf, &data_len, &complete); MPIU_ERR_CHKANDJUMP1(mpi_errno, mpi_errno, MPI_ERR_OTHER, "**ch3|postrecv", @@ -3703,39 +3696,32 @@ int MPIDI_CH3_PktHandler_Accumulate( MPIDI_VC_t *vc, MPIDI_CH3_Pkt_t *pkt, MPID_Datatype_get_size_macro(accum_pkt->datatype, type_size); req->dev.recv_data_sz = type_size * accum_pkt->count; - - if (req->dev.recv_data_sz == 0) { - MPIDI_CH3U_Request_complete(req); - *buflen = sizeof(MPIDI_CH3_Pkt_t); - *rreqp = NULL; - } - else { - mpi_errno = MPIDI_CH3U_Receive_data_found(req, data_buf, &data_len, - &complete); - MPIU_ERR_CHKANDJUMP1(mpi_errno, mpi_errno, MPI_ERR_OTHER, "**ch3|postrecv", - "**ch3|postrecv %s", "MPIDI_CH3_PKT_ACCUMULATE"); - /* FIXME: Only change the handling of completion if - post_data_receive reset the handler. There should - be a cleaner way to do this */ - if (!req->dev.OnDataAvail) { - req->dev.OnDataAvail = MPIDI_CH3_ReqHandler_PutAccumRespComplete; - } - /* return the number of bytes processed in this function */ - *buflen = data_len + sizeof(MPIDI_CH3_Pkt_t); - - if (complete) + + mpi_errno = MPIDI_CH3U_Receive_data_found(req, data_buf, &data_len, + &complete); + MPIU_ERR_CHKANDJUMP1(mpi_errno, mpi_errno, MPI_ERR_OTHER, "**ch3|postrecv", + "**ch3|postrecv %s", "MPIDI_CH3_PKT_ACCUMULATE"); + /* FIXME: Only change the handling of completion if + post_data_receive reset the handler. There should + be a cleaner way to do this */ + if (!req->dev.OnDataAvail) { + req->dev.OnDataAvail = MPIDI_CH3_ReqHandler_PutAccumRespComplete; + } + /* return the number of bytes processed in this function */ + *buflen = data_len + sizeof(MPIDI_CH3_Pkt_t); + + if (complete) + { + mpi_errno = MPIDI_CH3_ReqHandler_PutAccumRespComplete(vc, req, &complete); + if (mpi_errno) MPIU_ERR_POP(mpi_errno); + if (complete) { - mpi_errno = MPIDI_CH3_ReqHandler_PutAccumRespComplete(vc, req, &complete); - if (mpi_errno) MPIU_ERR_POP(mpi_errno); - if (complete) - { - *rreqp = NULL; - MPIU_INSTR_DURATION_END(rmapkt_acc_predef); - goto fn_exit; - } + *rreqp = NULL; + MPIU_INSTR_DURATION_END(rmapkt_acc_predef); + goto fn_exit; } - MPIU_INSTR_DURATION_END(rmapkt_acc_predef); - } + } + MPIU_INSTR_DURATION_END(rmapkt_acc_predef); } else { @@ -3831,52 +3817,45 @@ int MPIDI_CH3_PktHandler_Accumulate_Immed( MPIDI_VC_t *vc, MPIDI_CH3_Pkt_t *pkt, MPID_Datatype_get_extent_macro(accum_pkt->datatype, extent); - /* size == 0 should never happen */ - if (accum_pkt->count == 0 || extent == 0) { - ; + MPIU_INSTR_DURATION_START(rmapkt_acc_immed_op); + if (win_ptr->shm_allocated == TRUE) + MPIDI_CH3I_SHM_MUTEX_LOCK(win_ptr); + /* Data is already present */ + if (accum_pkt->op == MPI_REPLACE) { + /* no datatypes required */ + int len; + MPIU_Assign_trunc(len, (accum_pkt->count * extent), int); + /* FIXME: use immediate copy because this is short */ + MPIUI_Memcpy( accum_pkt->addr, accum_pkt->data, len ); } else { - MPIU_INSTR_DURATION_START(rmapkt_acc_immed_op); - if (win_ptr->shm_allocated == TRUE) - MPIDI_CH3I_SHM_MUTEX_LOCK(win_ptr); - /* Data is already present */ - if (accum_pkt->op == MPI_REPLACE) { - /* no datatypes required */ - int len; - MPIU_Assign_trunc(len, (accum_pkt->count * extent), int); - /* FIXME: use immediate copy because this is short */ - MPIUI_Memcpy( accum_pkt->addr, accum_pkt->data, len ); - } - else { - if (HANDLE_GET_KIND(accum_pkt->op) == HANDLE_KIND_BUILTIN) { - MPI_User_function *uop; - /* get the function by indexing into the op table */ - uop = MPIR_OP_HDL_TO_FN(accum_pkt->op); - (*uop)(accum_pkt->data, accum_pkt->addr, - &(accum_pkt->count), &(accum_pkt->datatype)); - } - else { - MPIU_ERR_SETANDJUMP1(mpi_errno,MPI_ERR_OP, "**opnotpredefined", - "**opnotpredefined %d", accum_pkt->op ); - } - } - if (win_ptr->shm_allocated == TRUE) - MPIDI_CH3I_SHM_MUTEX_UNLOCK(win_ptr); - MPIU_INSTR_DURATION_END(rmapkt_acc_immed_op); - + if (HANDLE_GET_KIND(accum_pkt->op) == HANDLE_KIND_BUILTIN) { + MPI_User_function *uop; + /* get the function by indexing into the op table */ + uop = MPIR_OP_HDL_TO_FN(accum_pkt->op); + (*uop)(accum_pkt->data, accum_pkt->addr, + &(accum_pkt->count), &(accum_pkt->datatype)); + } + else { + MPIU_ERR_SETANDJUMP1(mpi_errno,MPI_ERR_OP, "**opnotpredefined", + "**opnotpredefined %d", accum_pkt->op ); + } + } + if (win_ptr->shm_allocated == TRUE) + MPIDI_CH3I_SHM_MUTEX_UNLOCK(win_ptr); + MPIU_INSTR_DURATION_END(rmapkt_acc_immed_op); - /* There are additional steps to take if this is a passive - target RMA or the last operation from the source */ - - /* Here is the code executed in PutAccumRespComplete after the - accumulation operation */ - MPID_Win_get_ptr(accum_pkt->target_win_handle, win_ptr); + /* There are additional steps to take if this is a passive + target RMA or the last operation from the source */ - mpi_errno = MPIDI_CH3_Finish_rma_op_target(vc, win_ptr, TRUE, - accum_pkt->flags, - accum_pkt->source_win_handle); - if (mpi_errno) { MPIU_ERR_POP(mpi_errno); } - } + /* Here is the code executed in PutAccumRespComplete after the + accumulation operation */ + MPID_Win_get_ptr(accum_pkt->target_win_handle, win_ptr); + + mpi_errno = MPIDI_CH3_Finish_rma_op_target(vc, win_ptr, TRUE, + accum_pkt->flags, + accum_pkt->source_win_handle); + if (mpi_errno) { MPIU_ERR_POP(mpi_errno); } fn_exit: MPIU_INSTR_DURATION_END(rmapkt_acc_immed); @@ -4187,26 +4166,16 @@ int MPIDI_CH3_PktHandler_Get_AccumResp( MPIDI_VC_t *vc, MPIDI_CH3_Pkt_t *pkt, MPID_Datatype_get_size_macro(req->dev.datatype, type_size); req->dev.recv_data_sz = type_size * req->dev.user_count; - /* FIXME: It is likely that this cannot happen (never perform - a get with a 0-sized item). In that case, change this - to an MPIU_Assert (and do the same for accumulate and put) */ - if (req->dev.recv_data_sz == 0) { - MPIDI_CH3U_Request_complete( req ); - *buflen = sizeof(MPIDI_CH3_Pkt_t); + *rreqp = req; + mpi_errno = MPIDI_CH3U_Receive_data_found(req, data_buf, &data_len, &complete); + MPIU_ERR_CHKANDJUMP1(mpi_errno, mpi_errno, MPI_ERR_OTHER, "**ch3|postrecv", + "**ch3|postrecv %s", "MPIDI_CH3_PKT_GET_ACCUM_RESP"); + if (complete) { + MPIDI_CH3U_Request_complete(req); *rreqp = NULL; } - else { - *rreqp = req; - mpi_errno = MPIDI_CH3U_Receive_data_found(req, data_buf, &data_len, &complete); - MPIU_ERR_CHKANDJUMP1(mpi_errno, mpi_errno, MPI_ERR_OTHER, "**ch3|postrecv", - "**ch3|postrecv %s", "MPIDI_CH3_PKT_GET_ACCUM_RESP"); - if (complete) { - MPIDI_CH3U_Request_complete(req); - *rreqp = NULL; - } - /* return the number of bytes processed in this function */ - *buflen = data_len + sizeof(MPIDI_CH3_Pkt_t); - } + /* return the number of bytes processed in this function */ + *buflen = data_len + sizeof(MPIDI_CH3_Pkt_t); fn_exit: MPIU_INSTR_DURATION_END(rmapkt_get_accum); @@ -4392,38 +4361,30 @@ int MPIDI_CH3_PktHandler_LockPutUnlock( MPIDI_VC_t *vc, MPIDI_CH3_Pkt_t *pkt, req->dev.user_buf = new_ptr->pt_single_op->data; req->dev.lock_queue_entry = new_ptr; } - - if (req->dev.recv_data_sz == 0) { - *buflen = sizeof(MPIDI_CH3_Pkt_t); - MPIDI_CH3U_Request_complete(req); - *rreqp = NULL; + + int (*fcn)( MPIDI_VC_t *, struct MPID_Request *, int * ); + fcn = req->dev.OnDataAvail; + mpi_errno = MPIDI_CH3U_Receive_data_found(req, data_buf, &data_len, + &complete); + if (mpi_errno != MPI_SUCCESS) { + MPIU_ERR_SETFATALANDJUMP1(mpi_errno,MPI_ERR_OTHER, + "**ch3|postrecv", "**ch3|postrecv %s", + "MPIDI_CH3_PKT_LOCK_PUT_UNLOCK"); } - else { - int (*fcn)( MPIDI_VC_t *, struct MPID_Request *, int * ); - fcn = req->dev.OnDataAvail; - mpi_errno = MPIDI_CH3U_Receive_data_found(req, data_buf, &data_len, - &complete); - if (mpi_errno != MPI_SUCCESS) { - MPIU_ERR_SETFATALANDJUMP1(mpi_errno,MPI_ERR_OTHER, - "**ch3|postrecv", "**ch3|postrecv %s", - "MPIDI_CH3_PKT_LOCK_PUT_UNLOCK"); - } - req->dev.OnDataAvail = fcn; - *rreqp = req; + req->dev.OnDataAvail = fcn; + *rreqp = req; - if (complete) + if (complete) + { + mpi_errno = fcn(vc, req, &complete); + if (complete) { - mpi_errno = fcn(vc, req, &complete); - if (complete) - { - *rreqp = NULL; - } + *rreqp = NULL; } - - /* return the number of bytes processed in this function */ - *buflen = data_len + sizeof(MPIDI_CH3_Pkt_t); - } - + } + + /* return the number of bytes processed in this function */ + *buflen = data_len + sizeof(MPIDI_CH3_Pkt_t); if (mpi_errno != MPI_SUCCESS) { MPIU_ERR_SETFATALANDJUMP1(mpi_errno,MPI_ERR_OTHER, @@ -4650,34 +4611,27 @@ int MPIDI_CH3_PktHandler_LockAccumUnlock( MPIDI_VC_t *vc, MPIDI_CH3_Pkt_t *pkt, req->dev.lock_queue_entry = new_ptr; *rreqp = req; - if (req->dev.recv_data_sz == 0) { - *buflen = sizeof(MPIDI_CH3_Pkt_t); - MPIDI_CH3U_Request_complete(req); - *rreqp = NULL; + mpi_errno = MPIDI_CH3U_Receive_data_found(req, data_buf, &data_len, + &complete); + /* FIXME: Only change the handling of completion if + post_data_receive reset the handler. There should + be a cleaner way to do this */ + if (!req->dev.OnDataAvail) { + req->dev.OnDataAvail = MPIDI_CH3_ReqHandler_SinglePutAccumComplete; } - else { - mpi_errno = MPIDI_CH3U_Receive_data_found(req, data_buf, &data_len, - &complete); - /* FIXME: Only change the handling of completion if - post_data_receive reset the handler. There should - be a cleaner way to do this */ - if (!req->dev.OnDataAvail) { - req->dev.OnDataAvail = MPIDI_CH3_ReqHandler_SinglePutAccumComplete; - } - if (mpi_errno != MPI_SUCCESS) { - MPIU_ERR_SET1(mpi_errno,MPI_ERR_OTHER,"**ch3|postrecv", - "**ch3|postrecv %s", "MPIDI_CH3_PKT_LOCK_ACCUM_UNLOCK"); - } - /* return the number of bytes processed in this function */ - *buflen = data_len + sizeof(MPIDI_CH3_Pkt_t); + if (mpi_errno != MPI_SUCCESS) { + MPIU_ERR_SET1(mpi_errno,MPI_ERR_OTHER,"**ch3|postrecv", + "**ch3|postrecv %s", "MPIDI_CH3_PKT_LOCK_ACCUM_UNLOCK"); + } + /* return the number of bytes processed in this function */ + *buflen = data_len + sizeof(MPIDI_CH3_Pkt_t); - if (complete) + if (complete) + { + mpi_errno = MPIDI_CH3_ReqHandler_SinglePutAccumComplete(vc, req, &complete); + if (complete) { - mpi_errno = MPIDI_CH3_ReqHandler_SinglePutAccumComplete(vc, req, &complete); - if (complete) - { - *rreqp = NULL; - } + *rreqp = NULL; } } fn_fail: @@ -4714,27 +4668,18 @@ int MPIDI_CH3_PktHandler_GetResp( MPIDI_VC_t *vc ATTRIBUTE((unused)), MPID_Datatype_get_size_macro(req->dev.datatype, type_size); req->dev.recv_data_sz = type_size * req->dev.user_count; - /* FIXME: It is likely that this cannot happen (never perform - a get with a 0-sized item). In that case, change this - to an MPIU_Assert (and do the same for accumulate and put) */ - if (req->dev.recv_data_sz == 0) { - MPIDI_CH3U_Request_complete( req ); - *buflen = sizeof(MPIDI_CH3_Pkt_t); - *rreqp = NULL; - } - else { - *rreqp = req; - mpi_errno = MPIDI_CH3U_Receive_data_found(req, data_buf, - &data_len, &complete); - MPIU_ERR_CHKANDJUMP1(mpi_errno, mpi_errno, MPI_ERR_OTHER, "**ch3|postrecv", "**ch3|postrecv %s", "MPIDI_CH3_PKT_GET_RESP"); - if (complete) - { - MPIDI_CH3U_Request_complete(req); - *rreqp = NULL; - } - /* return the number of bytes processed in this function */ - *buflen = data_len + sizeof(MPIDI_CH3_Pkt_t); + *rreqp = req; + mpi_errno = MPIDI_CH3U_Receive_data_found(req, data_buf, + &data_len, &complete); + MPIU_ERR_CHKANDJUMP1(mpi_errno, mpi_errno, MPI_ERR_OTHER, "**ch3|postrecv", "**ch3|postrecv %s", "MPIDI_CH3_PKT_GET_RESP"); + if (complete) + { + MPIDI_CH3U_Request_complete(req); + *rreqp = NULL; } + /* return the number of bytes processed in this function */ + *buflen = data_len + sizeof(MPIDI_CH3_Pkt_t); + fn_exit: MPIDI_FUNC_EXIT(MPID_STATE_MPIDI_CH3_PKTHANDLER_GETRESP); return mpi_errno; ----------------------------------------------------------------------- Summary of changes: src/mpid/ch3/src/ch3u_rma_sync.c | 287 +++++++++++++++---------------------- test/mpi/rma/rmazero.c | 20 +++- 2 files changed, 135 insertions(+), 172 deletions(-) hooks/post-receive -- MPICH primary repository