Merge branch 'master' into 1575_fix

changelog

@@ -548,6 +548,10 @@
 	to prevent it breaking a line before the first non-whitepsace
 	character.
 
+	185) PR #2262 towards #446. Implement reference_access for Call nodes.
+
+	186) PR #2184 for #2166. Adds Sympy support for user-defined types.
+
 release 2.3.1 17th of June 2022
 
 	1) PR #1747 for #1720. Adds support for If blocks to PSyAD.

doc/developer_guide/sympy.rst

@@ -153,63 +153,27 @@ is case sensitive.
 
 User-defined Types
 ~~~~~~~~~~~~~~~~~~
-SymPy has no concept of user-defined types like
-``a(i)%b`` in Fortran. But this case is not handled especially, the
-PSyIR is converted to Fortran syntax and is provided unmodified to SymPy.
-SymPy interprets the ``%`` symbol
-as modulo function, so the expression above is read as ``Mod(a(i), b)``.
-This interpretation achieves the expected outcome when comparing structures
-and array references.
-For example, ``a(i+2*j-1)%b(k-i)`` and ``a(j*2-1+i)%b(-i+k)`` will be
-considered to be equal:
-
-1. Converting the two expressions to SymPy internally results in
-   ``Mod(a(i+2*j-1), b(k-i))`` and ``Mod(a(j*2-1+i, b(-i+k))``.
-2. Since nothing is known about the arguments of any of the ``Mod``
-   functions, SymPy will first detect that the same function is called
-   in both expression, and then continue to compare the arguments of
-   this function.
-3. The first arguments are ``a(i+2*j-1)`` and ``a(j*2-1+i)``.
-   The name ``a`` is considered an unknown function. SymPy detects
-   that both expressions appear to call the same function, and it
-   will therefore compare the arguments.
-4. SymPy compares ``i+2*j-1`` and ``j*2-1+i`` symbolically, and
-   evaluate these expressions to be identical. Therefore, the
-   two expressions ``a(...)`` are identical, so the first arguments
-   of the ``Mod`` function are identical.
-5. Similarly, it will then continue to evaluate the second argument
-   of the ``Mod`` function (``b(...)``), and evaluate them to be
-   identical.
-6. Since all arguments of the ``Mod`` function are identical,
-   SymPy will report these two functions to be the same, which
-   is the expected outcome.
-
-A member of a structure in Fortran becomes a stand-alone symbol (or
-function if it is an array) in SymPy. The SymPy
-writer will rename members to better indicate that they are members:
-an expression like ``a%b%c`` will be written as ``a%a_b%a_b_c``, which
-SymPy then parses as ``MOD(a, MOD(a_b, a_b_c))``. This convention
-makes it easier to identify what the various expressions in SymPy are.
-
-This handling of member variables can result in name clashes. Consider
-the expression ``a%b + a_b + b``. The structure access will be using
-two symbols ``a`` and ``a_b`` - but now there are two different symbols
-with the same name. Note that the renaming of the member from ``b`` to
-``a_b`` is not the reason for this - without renaming the same clash would
-happen with the symbol ``b``.
-
-The SymPy writer uses a symbol table to make sure it creates unique symbols.
+SymPy has no concept of user-defined types like ``a(i)%b`` in Fortran.
+A structure reference like this is converted to a single new symbol
+(scalar) or function (if an array index is involved). The default name
+will be the name of the reference and members concatenated using ``_``,
+e.g. ``a%b%c`` becomes ``a_b_c``, which will be declared as a new SymPy
+symbol or function (if it is an array access). The SymPy writer uses a
+symbol table to make sure it creates unique symbols.
 It first adds all References in the expression to the symbol table, which
-guarantees that no Reference to an existing symbol is renamed. The writer
-then renames all members and makes sure it uses a unique name. In the case of
-``a%b + a_b + b``, it would create ``a%a_b_1 + a_b + b``, using the name
-``a_b_1`` for the member to avoid the name clash with the reference
-``a_b`` - so an existing Reference will not be renamed, only members.
-
-.. note:: At this stage an expression using user-defined types cannot be
-    converted back to a PSyIR (which is what
-    `psyclone.core.SymbolicMaths.expand` does as a final step). This is
-    tracked as issue #2166.
+guarantees that no Reference to an existing symbol is renamed. In the case of
+``a%b + a_b + b``, it would create ``a_b_1 + a_b + b``, using the name
+``a_b_1`` for the structure reference to avoid the name clash with the
+reference ``a_b``.
+
+Any array indices are converted into arguments of this new function. So an
+expression like ``a(i)%b%c(j,k)`` becomes ``a_b_c(i,i,1,j,j,1,k,k,1)``
+(see :ref:`array_expressions`). The ``SymPyWriter`` creates a custom SymPy
+function, which keeps a list of which reference/member contained how many
+indices. In the example this would be ``[1, 0, 2]``, indicating that the
+first reference had one index, the second one none (i.e. it is not an
+array access), and the last reference had two indices. This allows the
+function to properly re-create the Fortran string.
 
 
 Documentation for SymPyWriter Functions

src/psyclone/psyad/transformations/preprocess.py

@@ -40,9 +40,8 @@
 
 '''
 from psyclone.core import SymbolicMaths
-from psyclone.psyad.utils import node_is_active, node_is_passive
-from psyclone.psyir.nodes import (BinaryOperation, Assignment, Reference,
-                                  StructureReference)
+from psyclone.psyad.utils import node_is_passive
+from psyclone.psyir.nodes import (Assignment, BinaryOperation, Reference)
 from psyclone.psyir.transformations import (DotProduct2CodeTrans,
                                             Matmul2CodeTrans,
                                             ArrayRange2LoopTrans,
@@ -101,94 +100,5 @@ def preprocess_trans(kernel_psyir, active_variable_names):
     # Deal with any associativity issues here as AssignmentTrans
     # is not able to.
     for assignment in kernel_psyir.walk(Assignment):
-        if assignment.walk(StructureReference):
-            # SymbolicMaths currently does not work if the expression
-            # contains user-defined types, see issue #2166.
-            associativity(assignment, active_variable_names)
-        else:
-            sym_maths = SymbolicMaths.get()
-            sym_maths.expand(assignment.rhs)
-
-
-def associativity(assignment, active_variable_names):
-    '''Repeatedly look for the patterns x * (a +- b) or (a +- b) */ x on
-    the rhs of this assignment where x is an inactive expression and a
-    and b are active expressions, replacing these patterns with x*a +-
-    x*b and a*/x +- b*/x respectively.
-
-    This function can be removed when support for Range nodes is added
-    to the SymbolicMaths expand function, see issue #1655.
-
-    :param assignment: the Assignment Node that we are looking at.
-    :type assignment: :py:class:`psyclone.psyir.nodes.Assignment`
-    :param active_variable_names: list of active variable names.
-    :type active_variable_names: list of str
-
-    '''
-    if node_is_active(assignment.rhs, active_variable_names):
-        while True:
-            for oper in assignment.rhs.walk(BinaryOperation):
-                # pylint: disable=too-many-boolean-expressions
-                if oper.operator == BinaryOperation.Operator.MUL and \
-                       node_is_passive(
-                           oper.children[0], active_variable_names) and \
-                       isinstance(oper.children[1], BinaryOperation) and \
-                       oper.children[1].operator in [
-                           BinaryOperation.Operator.ADD,
-                           BinaryOperation.Operator.SUB] and \
-                       node_is_active(
-                           oper.children[1].children[0],
-                           active_variable_names) and \
-                       node_is_active(
-                           oper.children[1].children[1],
-                           active_variable_names):
-                    # Matched one of the patterns we are looking for
-                    # x * (a +- b)
-                    inactive = oper.children[0]
-                    active0 = oper.children[1].children[0]
-                    active1 = oper.children[1].children[1]
-                    binary_op = oper.children[1]
-                    # Restructure to x*a +- x*b
-                    mult0 = BinaryOperation.create(
-                        BinaryOperation.Operator.MUL, inactive.detach(),
-                        active0.detach())
-                    mult1 = BinaryOperation.create(
-                        BinaryOperation.Operator.MUL, inactive.copy(),
-                        active1.detach())
-                    binary_op.children.extend([mult0, mult1])
-                    oper.replace_with(binary_op.detach())
-                    break
-
-                if oper.operator in [
-                        BinaryOperation.Operator.MUL,
-                        BinaryOperation.Operator.DIV] and \
-                        node_is_passive(
-                            oper.children[1], active_variable_names) and \
-                        isinstance(oper.children[0], BinaryOperation) and \
-                        oper.children[0].operator in [
-                            BinaryOperation.Operator.ADD,
-                            BinaryOperation.Operator.SUB] and \
-                        node_is_active(
-                            oper.children[0].children[0],
-                            active_variable_names) and \
-                        node_is_active(
-                            oper.children[0].children[1],
-                            active_variable_names):
-                    # Matched one of the patterns we are looking
-                    # for: (a +- b) */ x
-                    inactive = oper.children[1]
-                    active0 = oper.children[0].children[0]
-                    active1 = oper.children[0].children[1]
-                    binary_op = oper.children[0]
-                    # Restructure to a */ x +- b */ x
-                    op0 = BinaryOperation.create(
-                        oper.operator, active0.detach(), inactive.detach())
-                    op1 = BinaryOperation.create(
-                        oper.operator, active1.detach(), inactive.copy())
-                    binary_op.children.extend([op0, op1])
-                    oper.replace_with(binary_op.detach())
-                    break
-
-            else:
-                # No matching pattern so break out of while loop
-                break
+        sym_maths = SymbolicMaths.get()
+        sym_maths.expand(assignment.rhs)