Skip to content

Commit 23331a2

Browse files
sergisisosergisiso
committed
Merge remote-tracking branch 'origin/master' into loop_variable_as_child
2 parents 937cc7b + 40ae80c commit 23331a2

4 files changed

Lines changed: 287 additions & 168 deletions

File tree

‎changelog‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,6 @@
1+
3) PR #3480 for #3478. Updates OMPParallelLoopTrans to correctly
2+
use the kwargs approach.
3+
14
2) PR #3455 for #3170. Adds support for fields and operators with
25
explicit real32 and real64 kinds.
36

‎src/psyclone/psyir/transformations/omp_parallel_loop_trans.py‎

Lines changed: 31 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@
4343
import logging
4444
from typing import Iterable
4545
from psyclone.psyir.nodes import (
46-
OMPParallelDoDirective, OMPReductionClause)
46+
OMPParallelDoDirective, OMPReductionClause, Loop)
4747
from psyclone.psyir.nodes.omp_directives import MAP_REDUCTION_OP_TO_OMP
4848
from psyclone.psyir.transformations.omp_loop_trans import OMPLoopTrans
4949
from psyclone.utils import transformation_documentation_wrapper
@@ -87,8 +87,9 @@ class OMPParallelLoopTrans(OMPLoopTrans):
8787
def __str__(self):
8888
return "Add an 'OpenMP PARALLEL DO' directive"
8989

90-
def apply(self, node,
90+
def apply(self, node: Loop,
9191
force_private: Iterable[str] = tuple(),
92+
enable_reductions: bool = False,
9293
options=None, **kwargs):
9394
''' Apply an OMPParallelLoop Transformation to the supplied node
9495
(which must be a Loop). In the generated code this corresponds to
@@ -103,20 +104,41 @@ def apply(self, node,
103104
!$OMP END PARALLEL DO
104105
105106
:param node: the node (loop) to which to apply the transformation.
106-
:type node: :py:class:`psyclone.psyir.nodes.Loop`
107-
:param Iterable[str] force_private: specify a list of symbol names
107+
:param force_private: specify a list of symbol names
108108
explicitly requested to be private.
109+
:param enable_reductions: whether to enable PSyclone to compute
110+
reduction clauses on the parallelised loop.
109111
:param options: a dictionary with options for transformations
110112
and validation.
111113
:type options: Optional[Dict[str, Any]]
112114
'''
115+
logger = logging.getLogger(__name__)
116+
# TODO #2668 - deprecate options dictionary.
113117
local_options = options.copy() if options is not None else None
114-
if options and options.get("enable_reductions", False):
115-
local_options["reduction_ops"] = \
116-
list(MAP_REDUCTION_OP_TO_OMP.keys())
117-
118+
reduction_ops = []
119+
if options:
120+
enable_reductions = options.get("enable_reductions", False)
121+
if enable_reductions:
122+
local_options["reduction_ops"] = \
123+
list(MAP_REDUCTION_OP_TO_OMP.keys())
124+
else:
125+
if enable_reductions:
126+
# Reduction_ops isn't a supported option provided in this
127+
# Transformation's docstring, however since its in the
128+
# options for its superclass we give a warning and override
129+
# it as needed.
130+
if "reduction_ops" in kwargs:
131+
del kwargs["reduction_ops"]
132+
logger.warning(
133+
f"{self.name} overrides the provided reduction_ops "
134+
f"keyword argument to those supported by PSyclone."
135+
)
136+
reduction_ops = list(MAP_REDUCTION_OP_TO_OMP.keys())
137+
138+
# reduction_ops is the argument used by the superclass to determine
139+
# whether to allow reductions, so we don't pass enable_reductions.
118140
self.validate(node, options=local_options, force_private=force_private,
119-
**kwargs)
141+
reduction_ops=reduction_ops, **kwargs)
120142

121143
# keep a reference to the node's original parent and its index as these
122144
# are required and will change when we change the node's location
@@ -138,7 +160,6 @@ def apply(self, node,
138160
node_parent.addchild(directive, index=node_position)
139161

140162
# Add explicit private variables
141-
logger = logging.getLogger(__name__)
142163
explicitly_private_symbols = set()
143164
for symbol_name in force_private:
144165
try:

‎src/psyclone/tests/psyir/transformations/omp_parallelloop_trans_test.py‎

Lines changed: 250 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,250 @@
1+
# -----------------------------------------------------------------------------
2+
# BSD 3-Clause License
3+
#
4+
# Copyright (c) 2018-2026, Science and Technology Facilities Council.
5+
# All rights reserved.
6+
#
7+
# Redistribution and use in source and binary forms, with or without
8+
# modification, are permitted provided that the following conditions are met:
9+
#
10+
# * Redistributions of source code must retain the above copyright notice, this
11+
# list of conditions and the following disclaimer.
12+
#
13+
# * Redistributions in binary form must reproduce the above copyright notice,
14+
# this list of conditions and the following disclaimer in the documentation
15+
# and/or other materials provided with the distribution.
16+
#
17+
# * Neither the name of the copyright holder nor the names of its
18+
# contributors may be used to endorse or promote products derived from
19+
# this software without specific prior written permission.
20+
#
21+
# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
22+
# "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
23+
# LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS
24+
# FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE
25+
# COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT,
26+
# INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING,
27+
# BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
28+
# LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER
29+
# CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
30+
# LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN
31+
# ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
32+
# POSSIBILITY OF SUCH DAMAGE.
33+
# ----------------------------------------------------------------------------
34+
# Authors R. W. Ford, A. R. Porter and S. Siso, STFC Daresbury Lab
35+
# A. B. G. Chalk, STFC Daresbury Lab
36+
# Modified I. Kavcic, Met Office
37+
# Modified J. Henrichs, Bureau of Meteorology
38+
# Modified M. Naylor, University of Cambridge, UK
39+
40+
'''Contains the tests for the OMPParallelLoopTrans transformations.'''
41+
42+
import logging
43+
import pytest
44+
from psyclone.psyir.nodes import (
45+
CodeBlock, Literal, Loop)
46+
from psyclone.psyir.symbols import (
47+
ScalarType, DataSymbol)
48+
from psyclone.psyir.transformations import TransformationError
49+
from psyclone.tests.utilities import Compile
50+
from psyclone.transformations import OMPParallelLoopTrans
51+
52+
53+
def test_omplooptrans_apply_force_private(fortran_reader, fortran_writer,
54+
tmpdir, caplog):
55+
''' Test applying the OMPParallelLoopTrans in cases where a list of
56+
explicit private variables is given. '''
57+
58+
psyir = fortran_reader.psyir_from_source('''
59+
module my_mod
60+
contains
61+
subroutine my_subroutine()
62+
integer :: ji, jj, jk, jpkm1, jpjm1, jpim1, scalar1
63+
real, dimension(10, 10, 10) :: array1, array2
64+
array2 = 1
65+
do jk = 2, jpkm1, 1
66+
do jj = 2, jpjm1, 1
67+
do ji = 2, jpim1, 1
68+
array2(ji,jj,jk) = array2(ji,jj,jk) + 1
69+
array1(ji,jj,jk) = array2(ji,jj,jk)
70+
enddo
71+
enddo
72+
enddo
73+
end subroutine
74+
end module my_mod''')
75+
omplooptrans = OMPParallelLoopTrans()
76+
loop = psyir.walk(Loop)[0]
77+
78+
caplog.clear()
79+
with caplog.at_level(logging.WARNING,
80+
logger="psyclone.psyir.transformations"):
81+
omplooptrans.apply(loop, force_private=['array2', 'other'])
82+
83+
# 'other' is not used, but this is logged
84+
assert ("OMPParallelLoopTrans has been provided with the 'other' symbol "
85+
"name in the 'force_private' option, but there is no such symbol "
86+
"in this scope." in caplog.text)
87+
88+
# array2 is requested private, the shared_attibute_inference promotes it to
89+
# firstprivate because it is read first
90+
expected = '''\
91+
!$omp parallel do default(shared) private(ji,jj,jk) firstprivate(array2) \
92+
schedule(auto)
93+
do jk = 2, jpkm1, 1\n'''
94+
95+
gen = fortran_writer(psyir)
96+
assert expected in gen
97+
assert Compile(tmpdir).string_compiles(gen)
98+
99+
100+
def test_omplooptrans_apply_firstprivate(fortran_reader, fortran_writer,
101+
tmpdir):
102+
''' Test applying the OMPLoopTrans in cases where a firstprivate
103+
clause is needed to generate code that is functionally equivalent to the
104+
original, serial version.'''
105+
106+
# Example with a conditional write and a OMPParallelDoDirective
107+
psyir = fortran_reader.psyir_from_source('''
108+
module my_mod
109+
contains
110+
subroutine my_subroutine()
111+
integer :: ji, jj, jk, jpkm1, jpjm1, jpim1, scalar1, scalar2
112+
real, dimension(10, 10, 10) :: zwt, zwd, zwi, zws
113+
scalar1 = 1
114+
do jk = 2, jpkm1, 1
115+
do jj = 2, jpjm1, 1
116+
do ji = 2, jpim1, 1
117+
if (.true.) then
118+
scalar1 = zwt(ji,jj,jk)
119+
endif
120+
scalar2 = scalar1 + zwt(ji,jj,jk)
121+
zws(ji,jj,jk) = scalar2
122+
enddo
123+
enddo
124+
enddo
125+
end subroutine
126+
end module my_mod''')
127+
omplooptrans = OMPParallelLoopTrans()
128+
loop = psyir.walk(Loop)[0]
129+
omplooptrans.apply(loop)
130+
expected = '''\
131+
!$omp parallel do default(shared) private(ji,jj,jk,scalar2) \
132+
firstprivate(scalar1) schedule(auto)
133+
do jk = 2, jpkm1, 1
134+
do jj = 2, jpjm1, 1
135+
do ji = 2, jpim1, 1
136+
if (.true.) then
137+
scalar1 = zwt(ji,jj,jk)
138+
end if
139+
scalar2 = scalar1 + zwt(ji,jj,jk)
140+
zws(ji,jj,jk) = scalar2
141+
enddo
142+
enddo
143+
enddo
144+
!$omp end parallel do\n'''
145+
146+
gen = fortran_writer(psyir)
147+
assert expected in gen
148+
assert Compile(tmpdir).string_compiles(gen)
149+
150+
151+
def test_omplooptrans_apply_firstprivate_fail(fortran_reader):
152+
''' Test applying the OMPLoopTrans in cases where a firstprivate
153+
clause it is needed to generate functionally equivalent code than
154+
the starting serial version.
155+
156+
In some cases the transformation validate dependency analysis reports
157+
the firstprivate use as a reduction, which is wrong.
158+
159+
'''
160+
161+
# Example with a read before write and a OMPParallelDirective
162+
psyir = fortran_reader.psyir_from_source('''
163+
subroutine my_subroutine()
164+
integer :: ji, jj, jk, jpkm1, jpjm1, jpim1, scalar1, scalar2
165+
real, dimension(10, 10, 10) :: zwt, zwd, zwi, zws
166+
do jk = 2, jpkm1, 1
167+
do jj = 2, jpjm1, 1
168+
do ji = 2, jpim1, 1
169+
scalar2 = scalar1 + zwt(ji,jj,jk)
170+
scalar1 = 3
171+
zws(ji,jj,jk) = scalar2 + scalar1
172+
enddo
173+
enddo
174+
enddo
175+
end subroutine''')
176+
omplooptrans = OMPParallelLoopTrans()
177+
loop = psyir.walk(Loop)[0]
178+
try:
179+
omplooptrans.apply(loop)
180+
except TransformationError:
181+
# TODO #598: When this is solved, this test can be removed and the
182+
# "force":True in the previous test can also be removed
183+
pytest.xfail(reason="Issue #598: This example should be a firstprivate"
184+
" but the dependency analysis believes it is a "
185+
"reduction.")
186+
187+
188+
def test_parallellooptrans_refuse_codeblock():
189+
''' Check that ParallelLoopTrans.validate() rejects a loop nest that
190+
encloses a CodeBlock. We have to use OMPParallelLoopTrans as
191+
ParallelLoopTrans is abstract. '''
192+
otrans = OMPParallelLoopTrans()
193+
# Construct a valid Loop in the PSyIR with a CodeBlock in its body
194+
parent = Loop.create(DataSymbol("ji", ScalarType.integer_type()),
195+
Literal("1", ScalarType.integer_type()),
196+
Literal("10", ScalarType.integer_type()),
197+
Literal("1", ScalarType.integer_type()),
198+
[CodeBlock([], CodeBlock.Structure.STATEMENT,
199+
None)])
200+
with pytest.raises(TransformationError) as err:
201+
otrans.validate(parent)
202+
assert ("Nodes of type 'CodeBlock' cannot be enclosed "
203+
"by a OMPParallelLoopTrans transformation" in str(err.value))
204+
205+
206+
@pytest.mark.parametrize("use_options_dict", [True, False])
207+
def test_ompparallellooptrans_reductions(fortran_reader, fortran_writer,
208+
caplog, use_options_dict):
209+
'''Check that OMPParallelLoopTrans behaves correctly with reductions.
210+
The implementation is inherited from OMPLooptrans, however the options
211+
required differ for OMPParallelTrans (enable_reductions instead of
212+
reduction_ops).'''
213+
214+
code = """subroutine test
215+
integer :: a, i
216+
217+
do i = 1, 100
218+
a = a + i
219+
end do
220+
end subroutine test"""
221+
222+
psyir = fortran_reader.psyir_from_source(code)
223+
loop = psyir.walk(Loop)[0]
224+
if use_options_dict:
225+
OMPParallelLoopTrans().apply(
226+
loop, options={
227+
"enable_reductions": True, "reduction_ops": []
228+
})
229+
else:
230+
# If not using the options dict we should have a logged message too.
231+
caplog.clear()
232+
with caplog.at_level(logging.WARNING,
233+
"psyclone.psyir.transformations."
234+
"omp_parallel_loop_trans"):
235+
OMPParallelLoopTrans().apply(loop, enable_reductions=True,
236+
reduction_ops=[])
237+
assert caplog.records[0].levelname == "WARNING"
238+
assert ("OMPParallelLoopTrans overrides the provided "
239+
"reduction_ops keyword argument to those supported by "
240+
"PSyclone." in caplog.text)
241+
242+
correct = """!$omp parallel do default(shared) private(i) schedule(auto) \
243+
reduction(+: a)
244+
do i = 1, 100, 1
245+
a = a + i
246+
enddo
247+
!$omp end parallel do
248+
"""
249+
out = fortran_writer(psyir)
250+
assert correct in out

0 commit comments

Comments
 (0)