Skip to content

Commit cfd9f09

Browse files
committed
#3188 Test new privatisation validation
1 parent e4ebc9b commit cfd9f09

File tree

5 files changed

+59
-16
lines changed

5 files changed

+59
-16
lines changed

examples/nemo/scripts/omp_cpu_trans.py

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@
4040
import os
4141
from utils import (
4242
insert_explicit_loop_parallelism, normalise_loops, add_profiling,
43-
enhance_tree_information, PARALLELISATION_ISSUES, PRIVATISATION_ISSUES,
43+
enhance_tree_information, PARALLELISATION_ISSUES,
4444
NEMO_MODULES_TO_IMPORT)
4545
from psyclone.psyir.nodes import Routine
4646
from psyclone.transformations import OMPLoopTrans
@@ -112,7 +112,6 @@ def trans(psyir):
112112
region_directive_trans=omp_parallel_trans,
113113
loop_directive_trans=omp_loop_trans,
114114
collapse=False,
115-
privatise_arrays=(not NEMOV4 and
116-
psyir.name not in PRIVATISATION_ISSUES),
115+
privatise_arrays=not NEMOV4,
117116
enable_reductions=CPU_REDUCTIONS,
118117
)

examples/nemo/scripts/omp_gpu_trans.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@
4141
from utils import (
4242
add_profiling, inline_calls, insert_explicit_loop_parallelism,
4343
normalise_loops, enhance_tree_information, PARALLELISATION_ISSUES,
44-
NEMO_MODULES_TO_IMPORT, PRIVATISATION_ISSUES)
44+
NEMO_MODULES_TO_IMPORT)
4545
from psyclone.psyir.nodes import Routine
4646
from psyclone.psyir.transformations import (
4747
OMPTargetTrans, OMPDeclareTargetTrans)
@@ -236,7 +236,7 @@ def trans(psyir):
236236
region_directive_trans=omp_target_trans,
237237
loop_directive_trans=omp_gpu_loop_trans,
238238
collapse=True,
239-
privatise_arrays=(psyir.name not in PRIVATISATION_ISSUES),
239+
privatise_arrays=True,
240240
asynchronous_parallelism=ASYNC_PARALLEL,
241241
uniform_intrinsics_only=REPRODUCIBLE,
242242
enable_reductions=not REPRODUCIBLE
@@ -252,7 +252,7 @@ def trans(psyir):
252252
insert_explicit_loop_parallelism(
253253
subroutine,
254254
loop_directive_trans=omp_cpu_loop_trans,
255-
privatise_arrays=(psyir.name not in PRIVATISATION_ISSUES),
255+
privatise_arrays=True,
256256
asynchronous_parallelism=ASYNC_PARALLEL
257257
)
258258

examples/nemo/scripts/utils.py

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -180,10 +180,6 @@
180180
"traqsr.f90",
181181
]
182182

183-
PRIVATISATION_ISSUES = [
184-
"ldftra.f90", # Wrong runtime results
185-
]
186-
187183

188184
def _it_should_be(symbol, of_type, instance):
189185
''' Make sure that symbol has the datatype as provided.

src/psyclone/psyir/nodes/reference.py

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -300,6 +300,14 @@ def escapes_scope(
300300

301301
# Check if this instance is in the provided scope
302302
if not self.is_descendant_of(scope):
303+
# If the next_access is an array access we consider that it
304+
# has escaped the escope, because even if it is a write, it
305+
# can be to a subset of the indices. The rests will still
306+
# need the previous values.
307+
# pylint: disable=import-outside-toplevel
308+
from psyclone.psyir.nodes.array_mixin import ArrayMixin
309+
if isinstance(self, ArrayMixin):
310+
return True
303311
# If following the recursive calls through next_accesses()
304312
# it reaches a point outside the scope, return True (
305313
# it has escaped the scope), unless this is a write-only

src/psyclone/psyir/transformations/parallel_loop_trans.py

Lines changed: 46 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -49,9 +49,10 @@
4949
from psyclone.domain.common.psylayer import PSyLoop
5050
from psyclone.psyir import nodes
5151
from psyclone.psyir.nodes import (
52-
Call, Loop, Reference, Routine,
52+
Call, Loop, Reference, Routine, Assignment, IfBlock,
5353
BinaryOperation, IntrinsicCall
5454
)
55+
from psyclone.psyir.symbols import AutomaticInterface
5556
from psyclone.psyir.tools import (
5657
DependencyTools, DTCode, ReductionInferenceTool
5758
)
@@ -121,18 +122,57 @@ def _attempt_privatisation(loop, symbol_name, dry_run=False):
121122
# privatising these
122123
return False
123124

125+
# If it's not a local symbol, we cannot guarantee its lifetime
126+
if not isinstance(sym.interface, AutomaticInterface):
127+
return False
128+
124129
if sym in loop.explicitly_private_symbols:
125130
return True
126131

127-
# Check that the symbol is not referenced following this loop (before
128-
# the loop is fine because we can use OpenMP/OpenACC first-private or
129-
# Fortran do concurrent local_init())
130-
refs_in_loop = filter(lambda x: x.symbol is sym, loop.walk(Reference))
131-
last_access = list(refs_in_loop)[-1]
132132
loop.compute_cached_abs_positions()
133+
134+
# Get the last access
135+
refs_in_loop = filter(lambda x: x.symbol is sym, loop.walk(Reference))
136+
refs_in_loop = list(refs_in_loop)
137+
last_access = refs_in_loop[-1]
138+
# If it's an assignment the last access is the one in the lhs
139+
if last_access.ancestor(Assignment):
140+
lhs = last_access.ancestor(Assignment).lhs
141+
if isinstance(lhs, Reference) and lhs.symbol is sym:
142+
last_access = lhs
143+
# If the value of the symbol is used after the loop, it cannot be
144+
# private
133145
if last_access.escapes_scope(loop):
134146
return False
135147

148+
# Also prevent cases when the first access in the loop is a read that
149+
# could come from the previous iteration
150+
while True:
151+
first_access = refs_in_loop[0]
152+
# If it's an assignment the first access is the one in the rhs
153+
if first_access.ancestor(Assignment):
154+
rhs = first_access.ancestor(Assignment).rhs
155+
refs = filter(lambda x: x.symbol is sym, rhs.walk(Reference))
156+
refs = list(refs)
157+
if refs:
158+
first_access = refs[0]
159+
if first_access.is_read:
160+
return False
161+
# If it is inside a conditional, there may be more entry points for
162+
# this symbol, so we look for the next 'fist_access'
163+
inside_conditional = first_access.ancestor(IfBlock, limit=loop)
164+
if first_access.ancestor(IfBlock, limit=loop):
165+
following = inside_conditional.following_node()
166+
if not following:
167+
break
168+
# Skip al references in the condition that we already checked
169+
refs_in_loop = list(filter(
170+
lambda x: x.abs_position > following.abs_position,
171+
refs_in_loop
172+
))
173+
if not inside_conditional or not refs_in_loop:
174+
break
175+
136176
if not dry_run:
137177
loop.explicitly_private_symbols.add(sym)
138178

0 commit comments

Comments
 (0)