Merge pull request #2763 from stfc/2717_clause

Handle inconsistent OpenACC loop clauses

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

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):

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):

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()