Skip to content

Commit

Permalink
fix(tree): move declarations to top of function
Browse files Browse the repository at this point in the history
c++ suppoorts initializers anywhere in the function, but one must not
jump over an initializer with any goto usage. Given the lack of RAII in
C, this becomes a significant painpoint.

In large to-be-eventually-refactored functions contain gotos or use
switch statements, split declaration and initialization, and move all
declarations to the top of the function. This makes switch statements
and gotos safe in both languages.

stack-info: PR: #563, branch: aws-nslick/stack/10
Signed-off-by: Nicholas Sielicki <nslick@amazon.com>
  • Loading branch information
Nicholas Sielicki committed Sep 14, 2024
1 parent 4e64913 commit d769709
Show file tree
Hide file tree
Showing 6 changed files with 113 additions and 76 deletions.
37 changes: 19 additions & 18 deletions src/nccl_ofi_api.c
Original file line number Diff line number Diff line change
Expand Up @@ -318,15 +318,16 @@ ncclResult_t nccl_net_ofi_regMr(void *comm, void *data, size_t size, int type,
return ncclInternalError;
}

nccl_net_ofi_send_comm_t *send_comm = NULL;
nccl_net_ofi_recv_comm_t *recv_comm = NULL;

switch (base_comm->type) {
case NCCL_NET_OFI_SEND_COMM:;
nccl_net_ofi_send_comm_t *send_comm =
(nccl_net_ofi_send_comm_t *)base_comm;
case NCCL_NET_OFI_SEND_COMM:
send_comm = (nccl_net_ofi_send_comm_t *)base_comm;
ret = send_comm->regMr(send_comm, data, size, type, mhandle);
break;
case NCCL_NET_OFI_RECV_COMM:;
nccl_net_ofi_recv_comm_t *recv_comm =
(nccl_net_ofi_recv_comm_t *)base_comm;
case NCCL_NET_OFI_RECV_COMM:
recv_comm = (nccl_net_ofi_recv_comm_t *)base_comm;
ret = recv_comm->regMr(recv_comm, data, size, type, mhandle);
break;
default:
Expand All @@ -350,16 +351,16 @@ ncclResult_t nccl_net_ofi_deregMr(void *comm, void *mhandle)
}

int ret = 0;
nccl_net_ofi_send_comm_t *send_comm = NULL;
nccl_net_ofi_recv_comm_t *recv_comm = NULL;

switch (base_comm->type) {
case NCCL_NET_OFI_SEND_COMM:;
nccl_net_ofi_send_comm_t *send_comm =
(nccl_net_ofi_send_comm_t *)base_comm;
case NCCL_NET_OFI_SEND_COMM:
send_comm = (nccl_net_ofi_send_comm_t *)base_comm;
ret = send_comm->deregMr(send_comm, (nccl_net_ofi_mr_handle_t *)mhandle);
break;
case NCCL_NET_OFI_RECV_COMM:;
nccl_net_ofi_recv_comm_t *recv_comm =
(nccl_net_ofi_recv_comm_t *)base_comm;
case NCCL_NET_OFI_RECV_COMM:
recv_comm = (nccl_net_ofi_recv_comm_t *)base_comm;
ret = recv_comm->deregMr(recv_comm, (nccl_net_ofi_mr_handle_t *)mhandle);
break;
default:
Expand All @@ -386,17 +387,17 @@ ncclResult_t nccl_net_ofi_regMrDmaBuf(void* comm, void* data, size_t size,
}

int ret = 0;
nccl_net_ofi_send_comm_t *send_comm = NULL;
nccl_net_ofi_recv_comm_t *recv_comm = NULL;
nccl_net_ofi_mr_handle_t **handle = (nccl_net_ofi_mr_handle_t **)mhandle;

switch (base_comm->type) {
case NCCL_NET_OFI_SEND_COMM:;
nccl_net_ofi_send_comm_t *send_comm =
(nccl_net_ofi_send_comm_t *)base_comm;
case NCCL_NET_OFI_SEND_COMM:
send_comm = (nccl_net_ofi_send_comm_t *)base_comm;
ret = send_comm->regMrDmaBuf(send_comm, data, size, type, offset, fd, handle);
break;
case NCCL_NET_OFI_RECV_COMM:;
nccl_net_ofi_recv_comm_t *recv_comm =
(nccl_net_ofi_recv_comm_t *)base_comm;
case NCCL_NET_OFI_RECV_COMM:
recv_comm = (nccl_net_ofi_recv_comm_t *)base_comm;
ret = recv_comm->regMrDmaBuf(recv_comm, data, size, type, offset, fd, handle);
break;
default:
Expand Down
3 changes: 2 additions & 1 deletion src/nccl_ofi_ep_addr_list.c
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,7 @@ int nccl_ofi_ep_addr_list_insert(nccl_ofi_ep_addr_list_t *ep_list,
size_t addr_size)
{
int ret = 0;
ep_pair_list_elem_t *new_pair = NULL;

if (addr_size > ep_list->max_addr_size) {
NCCL_OFI_WARN("Address size %zu > max size (%zu)", addr_size,
Expand All @@ -155,7 +156,7 @@ int nccl_ofi_ep_addr_list_insert(nccl_ofi_ep_addr_list_t *ep_list,
memcpy(new_addr->addr, addr_in, addr_size);
zero_pad_address(new_addr->addr, addr_size, ep_list->max_addr_size);

ep_pair_list_elem_t *new_pair = (ep_pair_list_elem_t *)
new_pair = (ep_pair_list_elem_t*)
malloc(sizeof(*new_pair));
if (!new_pair) {
NCCL_OFI_WARN("Failed to allocate new ep list element");
Expand Down
5 changes: 3 additions & 2 deletions src/nccl_ofi_net.c
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,8 @@ int nccl_net_ofi_create_plugin(nccl_net_ofi_plugin_t **plugin_p)
{
int ret = 0;
const char *provider_filter = NULL;
nccl_net_ofi_ep_t *base_ep = NULL;
nccl_net_ofi_device_t *device = NULL;

NCCL_OFI_INFO(NCCL_INIT | NCCL_NET, "Initializing " PACKAGE_STRING);

Expand Down Expand Up @@ -230,8 +232,7 @@ int nccl_net_ofi_create_plugin(nccl_net_ofi_plugin_t **plugin_p)
* resources. This initialization happens once per process, and thus it
* does not matter which device is used to create the endpoint.
*/
nccl_net_ofi_device_t *device = (*plugin_p)->get_device(*plugin_p, 0);
nccl_net_ofi_ep_t *base_ep = NULL;
device = (*plugin_p)->get_device(*plugin_p, 0);

ret = device->get_ep(device, &base_ep);
if (ret != 0) {
Expand Down
Loading

0 comments on commit d769709

Please sign in to comment.