diff --git a/changelog b/changelog index 24a15f5876..56d32e873b 100644 --- a/changelog +++ b/changelog @@ -1,3 +1,7 @@ + 12) PR #3408 for #2812. Updates the Min/Max to code transformations so + that they use the datatype of their arguments rather than assuming real. + Also skips treesitter tests for Python < 3.10. + 11) PR #3414 for #3279. Update the lists of exclusions to improve GPU performance for NEMO. diff --git a/doc/user_guide/transformations.rst b/doc/user_guide/transformations.rst index 7fd87a0b41..f41f991468 100644 --- a/doc/user_guide/transformations.rst +++ b/doc/user_guide/transformations.rst @@ -344,11 +344,6 @@ can be found in the API-specific sections). :members: apply :no-index: -.. warning:: This transformation assumes that the MAX Intrinsic acts on - PSyIR Real scalar data and does not check that this is - not the case. Once issue #658 is on master then this - limitation can be fixed. - #### .. autoclass:: psyclone.psyir.transformations.Maxval2LoopTrans @@ -361,11 +356,6 @@ can be found in the API-specific sections). :members: apply :no-index: -.. warning:: This transformation assumes that the MIN Intrinsic acts on - PSyIR Real scalar data and does not check that this is - not the case. Once issue #658 is on master then this - limitation can be fixed. - #### .. autoclass:: psyclone.psyir.transformations.Minval2LoopTrans diff --git a/src/psyclone/psyir/transformations/intrinsics/minormax2code_trans.py b/src/psyclone/psyir/transformations/intrinsics/minormax2code_trans.py index 73aa14bcd6..42b1110e1c 100644 --- a/src/psyclone/psyir/transformations/intrinsics/minormax2code_trans.py +++ b/src/psyclone/psyir/transformations/intrinsics/minormax2code_trans.py @@ -47,11 +47,13 @@ from abc import ABC import warnings -from psyclone.psyir.nodes import BinaryOperation, Assignment, \ - Reference, IfBlock -from psyclone.psyir.symbols import DataSymbol, REAL_TYPE -from psyclone.psyir.transformations.intrinsics.intrinsic2code_trans import \ - Intrinsic2CodeTrans +from psyclone.psyir.nodes import ( + BinaryOperation, Assignment, Reference, IfBlock, IntrinsicCall +) +from psyclone.psyir.symbols import DataSymbol +from psyclone.psyir.transformations.intrinsics.intrinsic2code_trans import ( + Intrinsic2CodeTrans +) from psyclone.utils import transformation_documentation_wrapper @@ -85,7 +87,18 @@ def __init__(self): super().__init__() self._compare_operator = None - def apply(self, node, options=None, **kwargs): + def validate(self, node: IntrinsicCall, options=None, **kwargs): + ''' + Check that it is safe to apply the transformation to the supplied node. + + :param node: the SIGN call to transform. + :param options: any of options for the transformation. + + ''' + super().validate(node, options=options, **kwargs) + super()._validate_scalar_arg(node) + + def apply(self, node: IntrinsicCall, options=None, **kwargs): '''Apply this utility transformation to the specified node. This node must be a MIN or MAX IntrinsicCall. The intrinsic is converted to equivalent inline code. This is implemented as a PSyIR transform from: @@ -117,7 +130,6 @@ def apply(self, node, options=None, **kwargs): this is not the case. :param node: a MIN or MAX intrinsic. - :type node: :py:class:`psyclone.psyir.nodes.IntrinsicCall` :param options: a dictionary with options for transformations. :type options: Optional[Dict[str, Any]] @@ -132,21 +144,14 @@ def apply(self, node, options=None, **kwargs): symbol_table = node.scope.symbol_table assignment = node.ancestor(Assignment) - # Create a temporary result variable. There is an assumption - # here that the Intrinsic returns a PSyIR real type. This - # might not be what is wanted (e.g. the args might PSyIR - # integers), or there may be errors (arguments are of - # different types) but this can't be checked as we don't have - # appropriate methods to query nodes (see #658). + # Create two temporary variables. + result_type = node.arguments[0].datatype res_var_symbol = symbol_table.new_symbol( f"res_{self._intrinsic.name.lower()}", - symbol_type=DataSymbol, datatype=REAL_TYPE) - # Create a temporary variable. Again there is an - # assumption here about the datatype - please see previous - # comment (associated issue #658). + symbol_type=DataSymbol, datatype=result_type) tmp_var_symbol = symbol_table.new_symbol( f"tmp_{self._intrinsic.name.lower()}", - symbol_type=DataSymbol, datatype=REAL_TYPE) + symbol_type=DataSymbol, datatype=result_type) # Replace intrinsic with a temporary (res_var). node.replace_with(Reference(res_var_symbol)) diff --git a/src/psyclone/tests/psyir/frontend/fortran_test.py b/src/psyclone/tests/psyir/frontend/fortran_test.py index 5d873f5e96..ee40595372 100644 --- a/src/psyclone/tests/psyir/frontend/fortran_test.py +++ b/src/psyclone/tests/psyir/frontend/fortran_test.py @@ -38,6 +38,7 @@ ''' Performs py.test tests on the Fortran PSyIR front-end ''' import pytest + from psyclone.configuration import Config from psyclone.psyir.frontend.fortran import FortranReader from psyclone.psyir.frontend.fparser2 import Fparser2Reader @@ -49,6 +50,7 @@ from psyclone.psyir.commentable_mixin import CommentableMixin from psyclone.psyir.symbols import ( SymbolTable, DataSymbol, ScalarType, UnresolvedType) +from psyclone.tests.utilities import min_version_3_10 # The 'contiguous' keyword is just valid with Fortran 2008 @@ -87,6 +89,9 @@ ''' +# TODO #3416: Skip treesitter tests below 3.10 as they're unsupported by +# treesitter. +@min_version_3_10 def test_fortran_reader_constructor(): ''' Test that the constructor initialises the _parser and _processor attributes. ''' diff --git a/src/psyclone/tests/psyir/frontend/fortran_treesitter_reader/ftr_test.py b/src/psyclone/tests/psyir/frontend/fortran_treesitter_reader/ftr_test.py index 33705d3033..a92d6330d0 100644 --- a/src/psyclone/tests/psyir/frontend/fortran_treesitter_reader/ftr_test.py +++ b/src/psyclone/tests/psyir/frontend/fortran_treesitter_reader/ftr_test.py @@ -44,6 +44,7 @@ from psyclone.psyir.frontend.fortran_treesitter_reader import \ FortranTreeSitterReader from psyclone.psyir.nodes import FileContainer, CodeBlock, Container +from psyclone.tests.utilities import min_version_3_10 def test_constructor(): @@ -69,6 +70,9 @@ def test_constructor(): # TODO #3038 Typecheck arguments +# TODO #3416: Skip treesitter tests below 3.10 as they're unsupported by +# treesitter. +@min_version_3_10 def test_generate_parse_tree(tmpdir_factory, caplog): ''' Test that generate_parse_tree returns treesitter trees or appropriate @@ -92,7 +96,7 @@ def test_generate_parse_tree(tmpdir_factory, caplog): """ with pytest.raises(ValueError) as err: _ = processor.generate_parse_tree_from_source(invalid_code) - assert "Syntax Error found at line 2" in str(err.value) + assert "Syntax Error found at line" in str(err.value) # Test providing a source file filename = str(tmpdir_factory.mktemp('ts_test').join("testfile.f90")) @@ -115,6 +119,9 @@ def test_generate_parse_tree(tmpdir_factory, caplog): assert isinstance(ptree, TSNode) +# TODO #3416: Skip treesitter tests below 3.10 as they're unsupported by +# treesitter. +@min_version_3_10 def test_generate_psyir(): ''' Test that generate_psyir transforms treesitter parse trees to @@ -138,6 +145,9 @@ def test_generate_psyir(): assert isinstance(psyir.children[0].children[0], CodeBlock) +# TODO #3416: Skip treesitter tests below 3.10 as they're unsupported by +# treesitter. +@min_version_3_10 def test_codeblock_generation_and_messages(): ''' Test that NotImplementedErrors are caught and converted to CodeBlocks diff --git a/src/psyclone/tests/psyir/nodes/codeblock_test.py b/src/psyclone/tests/psyir/nodes/codeblock_test.py index a17f022e4a..39fb4640aa 100644 --- a/src/psyclone/tests/psyir/nodes/codeblock_test.py +++ b/src/psyclone/tests/psyir/nodes/codeblock_test.py @@ -39,6 +39,7 @@ ''' Performs py.test tests on the CodeBlock PSyIR node. ''' import pytest + from fparser.common.readfortran import FortranStringReader from psyclone.configuration import Config from psyclone.psyir.frontend.fortran import FortranReader @@ -50,8 +51,12 @@ ) from psyclone.psyir.nodes.node import colored from psyclone.errors import GenerationError, InternalError +from psyclone.tests.utilities import min_version_3_10 +# TODO #3416: Skip treesitter tests below 3.10 as they're unsupported by +# treesitter. +@min_version_3_10 def test_codeblock_create(): ''' Check the create method of the Code Block class.''' @@ -65,6 +70,13 @@ def test_codeblock_create(): assert isinstance(cb, Fparser2CodeBlock) assert "a => b" in cb.get_fortran_lines() + # Use a different fronted value + Config.get()._frontend = "newfrontend" + with pytest.raises(InternalError) as err: + cb = CodeBlock.create("3 + 3", partial_code="expression") + assert ("The 'newfrontend' frontend does not have an associated CodeBlock " + "subclass" in str(err.value)) + # Use the treesitter frontend (the frontend doesn't support partial # expressions yet, but it gets an appropriate error) Config.get()._frontend = "treesitter" @@ -77,13 +89,6 @@ def test_codeblock_create(): cb = CodeBlock.create("a => b", partial_code="pointer_assignment") assert "Syntax Error found at line 1: a => b" in str(err.value) - # Use a different fronted value - Config.get()._frontend = "newfrontend" - with pytest.raises(InternalError) as err: - cb = CodeBlock.create("3 + 3", partial_code="expression") - assert ("The 'newfrontend' frontend does not have an associated CodeBlock " - "subclass" in str(err.value)) - def test_codeblock_node_str(): ''' Check the node_str method of the Code Block class.''' @@ -170,6 +175,9 @@ def test_abstract_methods(): assert "Use appropriate CodeBlock subclass" in str(err.value) +# TODO #3416: Skip treesitter tests below 3.10 as they're unsupported by +# treesitter. +@min_version_3_10 def test_codeblock_get_fortran_lines(): ''' Test the get_fortran_lines method for fparser and treesiteer codeblocks. diff --git a/src/psyclone/tests/psyir/transformations/intrinsics/max2code_trans_test.py b/src/psyclone/tests/psyir/transformations/intrinsics/max2code_trans_test.py index b2dea24fac..38f07a3769 100644 --- a/src/psyclone/tests/psyir/transformations/intrinsics/max2code_trans_test.py +++ b/src/psyclone/tests/psyir/transformations/intrinsics/max2code_trans_test.py @@ -69,8 +69,8 @@ def test_apply(fortran_reader, fortran_writer): integer :: j integer :: k integer :: l - real :: res_max - real :: tmp_max + integer :: res_max + integer :: tmp_max res_max = i tmp_max = j diff --git a/src/psyclone/tests/psyir/transformations/intrinsics/min2code_trans_test.py b/src/psyclone/tests/psyir/transformations/intrinsics/min2code_trans_test.py index 82bd85d7a2..bfc2fd90e3 100644 --- a/src/psyclone/tests/psyir/transformations/intrinsics/min2code_trans_test.py +++ b/src/psyclone/tests/psyir/transformations/intrinsics/min2code_trans_test.py @@ -70,8 +70,8 @@ def test_apply(fortran_reader, fortran_writer): integer :: j integer :: k integer :: l - real :: res_min - real :: tmp_min + integer :: res_min + integer :: tmp_min res_min = i tmp_min = j diff --git a/src/psyclone/tests/utilities.py b/src/psyclone/tests/utilities.py index 0fb780beac..169441cfed 100644 --- a/src/psyclone/tests/utilities.py +++ b/src/psyclone/tests/utilities.py @@ -663,3 +663,9 @@ def make_external_module(monkeypatch, # that it will be found when the named module is requested. mman = ModuleManager.get() monkeypatch.setitem(mman._modules, mod_name, minfo) + + +# ============================================================================ +min_version_3_10 = pytest.mark.skipif( + sys.version_info < (3, 10), reason="tests require python 3.10 or higher" +)