Managing data movement: add information to data directive nodes and mv clause generation to visitor backend

[arporter]

In #1385 Sergi pointed out that the current approach (of generating the code for the clauses of an ACC DATA directive in a begin_string method inside the node) isn't really consistent with the visitor/backend code-generation step. He also suggested that we need to capture the variables that will be moved by a DATA directive as part of the Directive node (at the moment, we only generate this list at code-generation time). This will then allow us to examine neighbouring data Directives and optimise between them - i.e. removing unnecessary data movement.

[arporter]

Sergi also mentioned that in C++, giving a bare Reference to a DATA directive isn't sufficient because it's just a pointer without associated extent information. Therefore it has to be of the form my_array[0:N].

[arporter]

Since LFRic currently still relies on the gen_code code-generation path, any solution here needs to support both it and the visitor/backend path.

[arporter]

At the moment ACCDataDirective dynamically determines the contents of its various clauses (copy, copyin, copyout) at code-generation time. This means we don't know whether we need those clauses until code-generation time. I think the alternative is to move this functionality (of determining the required clauses) out of the directive class and into wherever we create the directive. Anything that modified the schedule containing such a directive would then have to ensure it was updated. This doesn't feel as good as what we currently have although, in practice, we would very rarely modify a schedule once data regions have been added.

[arporter]

I've now added some ACC clause classes and am experimenting with using them with ACCDataDirective. It almost worked out of the box which is very nice. However, now that I look at the code we currently have, it won't cope with complex structures where more than one Member is an array. From Dave Case's report on the NEMOVAR work, they had to do things like:

DO j_var = 1, pjb%fbge_vars%n3d 

  i_var = pjb%fbge_vars%getVar3d( j_var ) 

  !$ACC ENTER DATA IF(ll_transfer) & 

  !$ACC COPYIN(pjb%fbge%fbgemod_3d(i_var)) & 

  !$ACC COPYIN(pjb%fbge%fbgemod_3d(i_var)%fcor) 

  DO j_cov = 1, pjb%fbge%fbgemod_3d(i_var)%ncov 

  !$ACC ENTER DATA IF(ll_transfer)  COPYIN(pjb%fbge%fbgemod_3d(i_var)%fcor(j_cov)%fdif%cofsym_ipjk) 

[arporter]

i.e. it can't be done using a single DATA directive. For now I could simply refuse to deal with such cases but that's just delaying the inevitable. Perhaps the right answer is to let clauses contain array-range expressions and then expand these into explicit loops in a bespoke lower_to_language_level method? However, for the example above that would require e.g. pjb%fbge%fbgemod_3d(:)%fcore(:)%fdif%cofsym_ipjk and we've recently decided to forbid such things in the PSyIR because they aren't valid in Fortran.

[arporter]

That was a slightly arbitrary decision because obviously the PSyIR isn't Fortran so we could decide to support it.

[arporter]

I now have all the original tests passing with the new implementation. Since this issue is about adding clauses rather than supporting deep-copy of arbitrary derived types, I think I'll just add a check for the latter situation and raise an exception.

[arporter]

I lied when I said all the original tests are passing. I meant all the original NEMO tests. psyir_openacc_test.py(100)test_acc_data_region() is failing as it explicitly removes nodes from an existing data region and checks that the clauses are updated appropriately. Since I currently only set them up when the data node is created, this fails.

[arporter]

One option is to have some sort of Model-View pattern (I think?) where any changes to child nodes are propagated back up the tree with nodes updating as necessary. Something I'll need to talk to @sergisiso about when he's back.