Skip to content

Commit 00e4e7c

Browse files
committed
Add regression test for 172 bad co_reduce
[issue #172](#172) causes co_reduce to return wrong results if the binary operator has arguments declared with the `value` attribute rather than `intent(in)`. Switching to use `intent(in)` is a valid work around. The binary operator is required to be a pure function with two arguments. As @LadaF points out, F2008 says: > C1276 The specification-part of a pure function subprogram shall specify that all its nonpointer dummy data objects have the `INTENT(IN)` or the `VALUE` attribute. Original coverage diff from adding this test was at: https://codecov.io/gh/sourceryinstitute/opencoarrays/compare/882c371d4d7e84364eb7adfba4b4f8d840e3f398...1a0e3b7edb6867d9e9371cbd9ed1d8f5b3dd2010/changes If this commit produces a different change in coverage then that likely indicates where there is a bug in the library. If the library coverage remains the same, then the bug is probably in the compiler. See discussion at [#172](#172)
1 parent b9c2977 commit 00e4e7c

File tree

4 files changed

+71
-3
lines changed

4 files changed

+71
-3
lines changed

CMakeLists.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -435,6 +435,7 @@ if(opencoarrays_aware_compiler)
435435
endif()
436436
add_mpi_test(convert-before-put 3 ${tests_root}/regression/reported/convert-before-put)
437437
add_mpi_test(event-post 3 ${tests_root}/regression/reported/event-post)
438+
add_mpi_test(co_reduce-factorial 4 ${tests_root}/regression/reported/co_reduce-factorial)
438439
else()
439440
add_test(co_sum_extension ${tests_root}/unit/extensions/test-co_sum-extension.sh)
440441
set_property(TEST co_sum_extension PROPERTY PASS_REGULAR_EXPRESSION "Test passed.")

src/tests/regression/open/issue-172

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
../reported/issue-172-wrong-co_reduce.f90

src/tests/regression/reported/CMakeLists.txt

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,12 @@
1+
add_executable(co_reduce-factorial issue-172-wrong-co_reduce.f90)
2+
target_link_libraries(co_reduce-factorial OpenCoarrays)
3+
4+
add_executable(source-alloc-sync issue-243-source-allocation-no-sync.f90)
5+
target_link_libraries(source-alloc-sync OpenCoarrays)
6+
17
add_executable(convert-before-put issue-292-convert-type-before-put.f90)
28
target_link_libraries(convert-before-put OpenCoarrays)
39

410
add_executable(event-post issue-293-silent-event-failure.F90)
511
target_link_libraries(event-post OpenCoarrays)
612

7-
add_executable(source-alloc-sync issue-243-source-allocation-no-sync.f90)
8-
target_link_libraries(source-alloc-sync OpenCoarrays)
9-
Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
program co_reduce_factorial
2+
!! author: Daniel Topa & Izaak Beekman
3+
!! category: regression
4+
!!
5+
!! [issue #172](https://github.com/sourceryinstitute/opencoarrays/issues/172)
6+
!! wherein co-reduce gets junk in the first image when binary
7+
!! operator's (pure function) arguments have `value` attribute
8+
!! instead of `intent(in)`
9+
10+
implicit none
11+
integer :: value[ * ] !! Each image stores their image number here
12+
integer :: k
13+
value = this_image ( )
14+
call co_reduce ( value, result_image = 1, operator = myProd )
15+
!! value[k /= 1] undefined, value[ k == 1 ] should equal $n!$ where $n$ is `num_images()`
16+
if ( this_image ( ) == 1 ) then
17+
write ( * , '( "Number of images = ", g0 )' ) num_images ( )
18+
do k = 1, num_images ( )
19+
write ( * , '( 2( a, i0 ) )' ) 'value [ ', k, ' ] is ', value [ k ]
20+
write ( * , '(a)' ) 'since RESULT_IMAGE is present, value on other images is undefined by the standard'
21+
end do
22+
write ( * , '( "Product value = ", g0 )' ) value !! should print num_images() factorial
23+
write ( * , 100 )
24+
if ( value == factorial( num_images() ) ) then
25+
write ( * , '(a)' ) 'Test passed.'
26+
else
27+
write ( * , '(a, I0)') 'Answer should have been num_images()! = ', factorial( num_images() )
28+
error stop 'Wrong answer for n! using co_reduce'
29+
end if
30+
end if
31+
100 format ( "Expected value = num_images()!", /, " 2! = 2, 3! = 6, 4! = 24, ..." )
32+
33+
contains
34+
35+
pure function myProd ( a, b ) result ( rslt )
36+
!! Product function to be used in `co_reduce` reduction for
37+
!! computing factorials. When `value` attribute is changed to
38+
!! `intent(in)` tests pass, and expected behavior is observed.
39+
integer, value :: a, b
40+
!! multiply two inputs together. If we change `value` to
41+
!! `intent(in)` the test passes and the issue goes away and
42+
!! according to C1276 of F2008:
43+
!!
44+
!! > C1276 The specification-part of a pure function subprogram
45+
!! > shall specify that all its nonpointer dummy data objects have
46+
!! > the INTENT (IN) or the VALUE attribute.
47+
!!
48+
!! Thanks to @LadaF for pointing this out.
49+
integer :: rslt !! product of a*b
50+
rslt = a * b
51+
end function
52+
53+
pure function factorial ( n ) result ( rslt )
54+
!! Compute $n!$
55+
integer, intent(in) :: n
56+
integer :: rslt
57+
integer :: i
58+
rslt = 1
59+
do i = 1, n
60+
rslt = rslt*i
61+
end do
62+
end function
63+
end program

0 commit comments

Comments
 (0)