diff --git a/changelog b/changelog index 4991ac0d34..36c48af91b 100644 --- a/changelog +++ b/changelog @@ -266,6 +266,9 @@ API in PSyAD to base the generated test-code filenames on the name of the supplied LFRic Algorithm file. + 102) PR #2763 towards #2717. Upstreams PSyACC functionality: ensures + that clauses on an OpenACC directive are consistent. + release 2.5.0 14th of February 2024 1) PR #2199 for #2189. Fix bugs with missing maps in enter data diff --git a/psyclone.pdf b/psyclone.pdf index 89080a74e6..90b70d9774 100644 Binary files a/psyclone.pdf and b/psyclone.pdf differ diff --git a/src/psyclone/psyir/nodes/acc_directives.py b/src/psyclone/psyir/nodes/acc_directives.py index 72774dfc69..e4660caf6c 100644 --- a/src/psyclone/psyir/nodes/acc_directives.py +++ b/src/psyclone/psyir/nodes/acc_directives.py @@ -36,7 +36,7 @@ # C.M. Maynard, Met Office / University of Reading # J. Henrichs, Bureau of Meteorology # Modified A. B. G. Chalk, STFC Daresbury Lab -# Modified J. G. Wallwork, Met Office +# Modified J. G. Wallwork, Met Office / University of Cambridge # ----------------------------------------------------------------------------- ''' This module contains the implementation of the various OpenACC Directive @@ -447,6 +447,7 @@ def __init__(self, collapse=None, independent=True, sequential=False, self._sequential = sequential self._gang = gang self._vector = vector + self._check_clauses_consistent() super().__init__(**kwargs) def __eq__(self, other): @@ -469,6 +470,19 @@ def __eq__(self, other): return is_eq + def _check_clauses_consistent(self): + ''' + Check that the clauses applied to the loop directive make sense. + + :raises ValueError: if sequential is used in conjunction with gang + and/or vector + ''' + if self.sequential and (self.gang or self.vector): + raise ValueError( + "The OpenACC seq clause cannot be used in conjunction with the" + " gang or vector clauses." + ) + @property def collapse(self): ''' @@ -550,12 +564,13 @@ def node_str(self, colour=True): :returns: description of this node, possibly coloured. :rtype: str ''' + self._check_clauses_consistent() text = self.coloured_name(colour) - text += f"[sequential={self._sequential}," - text += f"gang={self._gang}," - text += f"vector={self._vector}," - text += f"collapse={self._collapse}," - text += f"independent={self._independent}]" + text += f"[sequential={self.sequential}," + text += f"gang={self.gang}," + text += f"vector={self.vector}," + text += f"collapse={self.collapse}," + text += f"independent={self.independent}]" return text def validate_global_constraints(self): @@ -619,17 +634,18 @@ def begin_string(self, leading_acc=True): if leading_acc: clauses = ["acc", "loop"] - if self._sequential: + self._check_clauses_consistent() + if self.sequential: clauses += ["seq"] else: - if self._gang: + if self.gang: clauses += ["gang"] - if self._vector: + if self.vector: clauses += ["vector"] - if self._independent: + if self.independent: clauses += ["independent"] - if self._collapse: - clauses += [f"collapse({self._collapse})"] + if self.collapse: + clauses += [f"collapse({self.collapse})"] return " ".join(clauses) def end_string(self): diff --git a/src/psyclone/tests/psyir/backend/psyir_openacc_test.py b/src/psyclone/tests/psyir/backend/psyir_openacc_test.py index 2474e289ea..4ebf310699 100644 --- a/src/psyclone/tests/psyir/backend/psyir_openacc_test.py +++ b/src/psyclone/tests/psyir/backend/psyir_openacc_test.py @@ -33,7 +33,7 @@ # ----------------------------------------------------------------------------- # Author: A. R. Porter, STFC Daresbury Lab # Modified by: S. Siso and N. Nobre, STFC Daresbury Lab -# Modified by: J. G. Wallwork, Met Office +# Modified by: J. G. Wallwork, Met Office / University of Cambridge # ----------------------------------------------------------------------------- '''Performs pytest tests on the support for OpenACC directives in the @@ -246,6 +246,13 @@ def test_acc_loop(fortran_reader, fortran_writer): " !$acc loop gang vector\n" " do jj = 1, jpj, 1\n" in result) + # Check clause inconsistencies are caught when using the apply method + for clause in ("gang", "vector"): + with pytest.raises(ValueError) as err: + acc_trans.apply(loops[0], {"sequential": True, clause: True}) + assert ("The OpenACC seq clause cannot be used in conjunction with the" + " gang or vector clauses." in str(err.value)) + # ---------------------------------------------------------------------------- def replace_child_with_assignment(node): diff --git a/src/psyclone/tests/psyir/nodes/acc_directives_test.py b/src/psyclone/tests/psyir/nodes/acc_directives_test.py index f3d3191d85..b817b63353 100644 --- a/src/psyclone/tests/psyir/nodes/acc_directives_test.py +++ b/src/psyclone/tests/psyir/nodes/acc_directives_test.py @@ -34,7 +34,7 @@ # Authors R. W. Ford, A. R. Porter, S. Siso and N. Nobre, STFC Daresbury Lab # Modified I. Kavcic, Met Office # Modified A. B. G. Chalk, STFC Daresbury Lab -# Modified J. G. Wallwork, Met Office +# Modified J. G. Wallwork, Met Office / University of Cambridge # ----------------------------------------------------------------------------- ''' Performs py.test tests on the OpenACC PSyIR Directive nodes. ''' @@ -223,8 +223,10 @@ def test_accenterdatadirective_gencode_4(trans1, trans2): # Class ACCLoopDirective start -def test_accloopdirective_node_str(monkeypatch): - ''' Test the node_str() method of ACCLoopDirective node ''' +def test_accloopdirective_node_str_default(monkeypatch): + ''' + Test the node_str() method of ACCLoopDirective node with default arguments. + ''' directive = ACCLoopDirective() # Mock the coloured name as this is tested elsewhere @@ -237,18 +239,48 @@ def test_accloopdirective_node_str(monkeypatch): assert directive.node_str() == expected assert str(directive) == expected + +def test_accloopdirective_node_str_nondefault(monkeypatch): + ''' + Test the node_str() method of ACCLoopDirective node with non-default + arguments. + ''' + directive = ACCLoopDirective( + sequential=False, collapse=2, independent=False, gang=True, vector=True + ) + + # Mock the coloured name as this is tested elsewhere + monkeypatch.setattr(directive, "coloured_name", + lambda x: "ACCLoopDirective") + + # Non-default value output + expected = ("ACCLoopDirective[sequential=False,gang=True,vector=True," + "collapse=2,independent=False]") + assert directive.node_str() == expected + assert str(directive) == expected + # Non-default value output directive._sequential = True - directive._collapse = 2 - directive._independent = False - directive._gang = True - directive._vector = True - expected = ("ACCLoopDirective[sequential=True,gang=True,vector=True," + directive._gang = False + directive._vector = False + expected = ("ACCLoopDirective[sequential=True,gang=False,vector=False," "collapse=2,independent=False]") assert directive.node_str() == expected assert str(directive) == expected +def test_accloopdirective_inconsistent_clause_error(): + ''' + Test the ACCLoopDirective constructor raises a ValueError if inconsistent + clause arguments are passed. + ''' + for kwargs in ({"gang": True}, {"vector": True}): + with pytest.raises(ValueError) as err: + _ = ACCLoopDirective(sequential=True, **kwargs) + assert ("The OpenACC seq clause cannot be used in conjunction with the" + " gang or vector clauses." in str(err.value)) + + def test_accloopdirective_collapse_getter_and_setter(): ''' Test the ACCLoopDirective collapse property setter and getter.''' target = ACCLoopDirective()