(#Towards 3420) Move OMPParallelTrans to create the data sharing attributes at transformation time

[LonelyCat124]

I have a draft implementation of moving OMPParallelTrans to do clause generation (using logic that is still located in the directive, so can be called independently if the transformation isn't used for whatever reason). Reduction logic isn't all available at transformation time, so I've not touched that yet (maybe when Kern <=> Call is done we can revisit that?)

@arporter @sergisiso If either of you could have a look and let me know i fyou think I've overlooked anything with this basic change before I look at some of the other potential improvements we can do as well. I'm going to set ITs going as well to see if this breaks any of the transformation scripts/runtime behaviour of real codes too.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.96%. Comparing base (ca8610a) to head (5e1ceb2).
⚠️ Report is 27 commits behind head on master.

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #3421   +/-   ##
=======================================
  Coverage   99.96%   99.96%           
=======================================
  Files         391      391           
  Lines       54609    54627   +18     
=======================================
+ Hits        54590    54608   +18     
  Misses         19       19           

☔ View full report in Codecov by Sentry.
📢 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.

[sergisiso]

@LonelyCat124 I have a couple of questions

1. the NEMO integration tests passed by diffing with the previous version it seems that is missing the private attribute for loop variable indices:

>     !$omp teams loop collapse(2) default(shared) private(ji,jj)
870c870
<       !$omp teams loop collapse(2) default(shared)
---
>       !$omp teams loop collapse(2) default(shared) private(ji,jj)
888c888
<     !$omp teams loop collapse(2) default(shared)
---
>     !$omp teams loop collapse(2) default(shared) private(idx_4,idx_5)
904c904
<     !$omp teams loop collapse(2) default(shared)
---
>     !$omp teams loop collapse(2) default(shared) private(idx_6,idx_7)
922c922
<     !$omp teams loop collapse(2) default(shared)
---
>     !$omp teams loop collapse(2) default(shared) private(ji,jj)
1024c1024
<       !$omp teams loop collapse(2) default(shared)

Is this valid with a default(shared)? can we recover them?

2. The infer_sharing_attributes is still done in the lowering, could we avoid it? It is the larger contributor to the backend time:

[LonelyCat124]

@sergisiso Ready for another look now.

I think 1) is fixed - I'd missed some cases that I now catch.

2. I think we can't yet - the reason is we need the need_sync variables in lowering for adding reduction clauses, which we can't do a transformation time due to LFRicKern (or Kern in general) and reproducible reductions I think.

[sergisiso]

the reason is we need the need_sync variables in lowering for adding reduction clauses, which we can't do a transformation time due to LFRicKern (or Kern in general) and reproducible reductions I think.

Is this because Kern does not report its used symbols in the reference_accesses that the infer_sharing_attributes uses to do its analysis? That may not be a problem (in the future) as providing them as virtual_children as we do in GOcean Kerns would fix it and is something that we plan to do anyway to make the DUC work with them to replace the old halo_exchange analysis.

But, is there a more fundamental problem that ANY symbol/reference that is created at lowering (if we don't repeat the infer_sharing_attributes) will become shared without synchronisation (the default)? I am not sure how I feel about this yet.

[sergisiso]

I think the list of aspects that we need to evaluate are:

1. Check if it makes the decisions regarding which loops are parallel and which variables are private easier if they are done together instead of in very different places.
2. Check if not repeating the same analysis in two different places (during transformation and after lowering) also prevents issues caused if we get different results (but we lose the ability to be smart about changes done latter, e.g in posterior transformations or new variables introduced during lowering ).
3. Check if doing it the associated reference_access analysis once is noticeably faster as it is potentially a expensive analysis (but with the current hoisting is not a dominating factor).

This won't be possible only we really can skip the lowering call we need for the reductions, so I would block this PR until we do the Kern reference_access children.