Permit implicit reshaping in get_callee()

[mn416]

Currently, get_callee() requires the ranks of an array argument to match at the caller and callee but this is not strictly necessary in Fortran due to implicit reshaping (which occurs in UKCA). This PR requires the ranks to match only if:

1. the call is to a GenericInterfaceSymbol rather than a RoutineSymbol, or
2. the dummy argument is not an explicit-shape array.

I appreciate that implicit reshaping may not be considered good practice - perhaps it is a deliberate decision to ignore it? If not, perhaps this PR is useful.

[LonelyCat124]

@sergisiso This probably looks like it'd be best for you to review (as you may know more about if/why we may not support this at current). If I'm wrong and you're too busy then let me know and I'm also happy to have a look. I'm not sure if/how this could affect inlining (if it would at all, but inlining functions with this behaviour would be bad)?

[arporter]

@sergisiso This probably looks like it'd be best for you to review (as you may know more about if/why we may not support this at current). If I'm wrong and you're too busy then let me know and I'm also happy to have a look. I'm not sure if/how this could affect inlining (if it would at all, but inlining functions with this behaviour would be bad)?

Exactly - this functionality was (I think) driven by the inlining use case and therefore I think that's the reason I wrote it this way. If it would be useful to have the option to permit re-shaped arguments then I don't have a problem with that, as long as it can still be refused when trying to inline.

[mn416]

Hi @arporter and @LonelyCat124, I believe that InlineTrans already checks that the formal and actual ranks are equal and raises and error if not:

https://github.com/stfc/PSyclone/blob/master/src/psyclone/psyir/transformations/inline_trans.py#L1239-L1249

Indeed, this feels like a much better place for the rank check. I noticed it because I had to tweak one of the inlining tests (see "Files Changed") which, after my proposed change, hits a different (and clearer) error message about InlineTrans disallowing array reshaping (the old error message was about the target of the call not being found).

Can you think of any other blockers for this PR? Unfortunately, implicit reshaping does crop up in important places in UKCA, and PSyclone not resolving the call is blocking some of my analyses.

Edit: I may be able to update UKCA to avoid implicit reshaping in some places, but I am still thinking that the PR could be an improvement -- it is just allowing something that Fortran allows.

[sergisiso]

Sorry for not replying earlier, in short I do agree this is an improvement specially if it unblocks work on UKCA.

I got distracted because I was considering:

• if we could potentially fix the inlining by creating a temporary array and assigning to it with and explicit reshape instrinsic.
• reading the "sequence association" section from the Fortran standard the associations could be much wider than arrays of different shape. Any reference to an array can be taken as a starting point of the subroutine argument array, for example this is valid Fortran:

program test
    integer, dimension(4) :: a = (/1,2,3,4/)
    call mysub(a(2))
    contains
    subroutine mysub(b)
        integer, dimension(3) :: b
        write(*,*) b
    end subroutine
end program

But none of these need to be fixed now for this PR to progress.

[arporter]

Thanks very much Matthew. I've done a first pass and there are just comments and docstrings to tweak. Once you've fixed the conflict and the tests, I'll check the integration tests too.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (0194bcb) to head (b0507f1).
⚠️ Report is 6 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff            @@
##            master     #3442   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files          393       393           
  Lines        55065     55067    +2     
=========================================
+ Hits         55065     55067    +2     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[mn416]

Thanks @arporter. I've brought it up-to-date with master and made the suggested changes.

[arporter]

Changes all look good now, thanks very much. Just waiting on the ITs...

[arporter]

ITs were all green. This is good to go.