#172 Fix calling functions with value arguments in co_reduce #331
Conversation
Add support for recude-functions with value parameters. Add support for char-arrays as arguments to reduce-functions. Rename co_reduce_1 to internal_co_reduce to get a better naming. Create mpi-datatype to transport the char-array length. Fixes #172.
|
Andre Vehreschild seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. |
|
Ignore! |
There was a problem hiding this comment.
@vehre Thanks so much for this PR. This is a CRITICAL bug fix, IMO, and it's great to be finally resolving it.
Can you please associate the email address used for that commit with your github account to satisfy the CLA compliance bot?
Also, if you can address some of my comments I'd appreciate it. If not, I'll rebase/cleanup myself once you're ready.
| # remove this before merging into master | ||
| set_property(TEST co_reduce-factorial PROPERTY WILL_FAIL TRUE) | ||
| # set_property(TEST co_reduce-factorial PROPERTY WILL_FAIL TRUE) |
There was a problem hiding this comment.
Just delete this line entirely please.
| co_reduce_1 (MPI_Op op, gfc_descriptor_t *source, int result_image, int *stat, | ||
| char *errmsg, int src_len __attribute__ ((unused)), int errmsg_len) | ||
| internal_co_reduce (MPI_Op op, gfc_descriptor_t *source, int result_image, int *stat, | ||
| char *errmsg, int src_len, int errmsg_len) |
There was a problem hiding this comment.
Is it possible to set your editor to use spaces and not tab characters? Also, it would be great to revert the leading indentation whitespace changes, or at least fixup white space in a separate commit from when substantive edits are made.
| @@ -3285,17 +3327,17 @@ co_reduce_1 (MPI_Op op, gfc_descriptor_t *source, int result_image, int *stat, | |||
| if (rank == 0 || PREFIX (is_contiguous) (source)) | |||
| { | |||
| if (result_image == 0) | |||
| ierr = MPI_Allreduce (MPI_IN_PLACE, source->base_addr, size, datatype, | |||
| ierr = MPI_Allreduce (MPI_IN_PLACE, source->base_addr, size, datatype, | |||
There was a problem hiding this comment.
please revert whitespace only changes by amending the commit if you know how... if not, I'll do it.
| result_image-1, CAF_COMM_WORLD); | ||
| else | ||
| ierr = MPI_Reduce (source->base_addr, NULL, size, datatype, op, | ||
| ierr = MPI_Reduce (source->base_addr, NULL, size, datatype, op, |
There was a problem hiding this comment.
Same for these two diffs, whitespace only changes...
lol OK btw if you add the WIP (work in progress) label, then you can leave the pull request open and keep working on it and pushing commits, without any danger of someone merging it. Also add that to the title to be safe... then you can just leave it open... |
Fixing co_reduce with value argumented functions and char arrays as arguments.
Fixing co_broadcast cleanup.
Added some testcases.
Fixes #172