PSyclone icon indicating copy to clipboard operation
PSyclone copied to clipboard

Incomplete variable access information for LFRic kernels, e.g. in LoopFusion

Open hiker opened this issue 1 year ago • 4 comments

The reference_accesses methods for an LFRic kernel adds the arguments to the access list, but (esp. in the case of fields) does not provide any index information. As a result of this, the dependency_analysis will not work properly. For example, in (the new #2488) loop_fusion additional checks for validity will not work with LFRic (and have de-facto been disabled for any PSyLoop, i.e. for any LFRic or gocean kernel):

FAILED psyGen_test.py::test_args_filter - psyclone.psyir.transformations.transformation_error.TransformationError: Transformation Error: Variable 'f1_data' does not depend on loop variable 'cell', but is read and written

Which is basically because the access to f1_data does not have any index information. This would also potentially affect the DependencyAnalysis tools.

I am adding a #TODO to the new loop_fusion for this issue. The test code seems to work with gocean once #2496 is merged, so we either need a way for lfric loop fusion to disable this test, or add index information. The latter would be the preferred option, but it's very hard to given the indirect accesses used in LFRic to handle this properly - the loop index is typically used to select the mapping values to use:

      DO cell=loop0_start,loop0_stop
        CALL summation_w0_to_w3_kernel_code(nlayers, field_3_data..., map_w3(:,cell), ndf_w0, undf_w0, map_w0(:,cell))

And in the kernel:

      do k=0, nlayers-1
        do i=1, ndf_w0
          field_w3(map_w3(1)+k) = field_w3(map_w3(1)+k) + field_w0(map_w0(i)+k)

So even if we would look into the kernel, the dependency of the indices used for (say) field_w3 on cell is not obvious. Maybe we could add artificial accesses (like we do for gocean kernels, though in this case there is no indirect addressing), i.e. add field_w3(cell) just as an artificial access??

hiker avatar Feb 07 '24 03:02 hiker