PSyclone icon indicating copy to clipboard operation
PSyclone copied to clipboard

Add Options to Variable Usage Functions

Open hiker opened this issue 2 years ago • 3 comments

#1742 shows that in some cases for example the variables used in defining the loop boundaries should not be included in the list of 'read' variables (size(some_array,1) - does not really read some_array). In other cases, the loop boundaries are required (kernel extraction), and/or should even include functions (do i = 2, self%vspace%get_last_dof_owned()).

My suggestion: we add an optional option-dictionary, similar to what we do in some of the apply and validate functions, where the calling function can influence the behaviour of the variable usage collection. E.g.:

reference.reference_accesses(var_access_info, {"loop-bounds": None})
reference.reference_accesses(var_access_info, {"loop-bounds": "variables"})
reference.reference_accesses(var_access_info, {"loop-bounds": "include-functions"})

The first call would not include any variables used in a loop boundary specification, the second one would be the current behaviour (get variables, but not function calls), the third one would also include function calls.

hiker avatar Jun 09 '22 02:06 hiker

We may be talking about 2 different things here:

  • The reference_accesses of the whole loop, which includes a read of the boundaries at the beginning of the execution.
  • The single-iteration reference access, which is what we need for the dependency analysis and in Fortran does not involve a read of the boundaries.

Maybe for the dependency analysis it should be doing loop_body.reference_accesses(var_access_info) and for the kernel extraction loop.reference_accesses(var_access_info)?

sergisiso avatar Jun 10 '22 09:06 sergisiso

I have started to implement the following option:

    :param bool array_shape_accesses_are_read: if True (default), an access \
        to an array using `size`, `lbound`, or `ubound` are considered read \
        accesses  to the array. If this is set to False, these accesses will \
        not be reported as 'read' accesses.
...    def __init__(self, nodes=None, array_shape_accesses_are_read=True):

So basically, if you set this variable to false, then the functions size, lbound and ubound will not report the array as being read anymore. This should fix this problem (and still allow the kernel extraction to be aware of the usage in the loop bounds).

I did discover that in https://github.com/stfc/PSyclone/pull/829/commits/b973396ba716d4178ad7c456ce7df25e78ed7360 code was added to disable reporting for lbound and ubound in the Reference object. This was apparently required for openacc. I am trying to remove this code and rely ont his new feature instead (the old way would not report a read in all cases, e.g. lbound(array, my_dim), i.e. when the dimension is a variable).

hiker avatar Jun 27 '22 07:06 hiker

In #1840 a patch is shown that removes a crash if a whole-array access is used. This seems to be a useful fix that should be integrated here as well.

hiker avatar Aug 15 '22 03:08 hiker

Did #1852 cover this issue @hiker? (i.e. can it now be closed?)

arporter avatar Oct 19 '22 11:10 arporter

Yes, I think that's good enough. More detailed options can easily be added if required, afaik we have all actual use cases covered.

hiker avatar Oct 19 '22 13:10 hiker