Unnecessary OMP first private clause being added to OMP directive around UM loop

[MetBenjaminWent]

Whilst psycloning a UM source code file mphys_air_density_mod.F90, around the loop found between lines 315-402, an unnecessary first private clause is being added to the OMP directive. The OMP around the loop has been generated by the formally known as NEMO API, which is using the latest psyclone release.
https://code.metoffice.gov.uk/trac/um/browser/main/trunk/src/atmosphere/large_scale_precipitation/mphys_air_density_mod.F90

The generated OMP directive looks like:
!$omp parallel do collapse(3) default(shared), private(i,j,k,q_total,rho1), firstprivate(rho2), schedule(static)

rho2 has not been intitalised or used above, only declared at the start of the file.

This is currently causing build failures with the CCE compiler on the Quads, which is spotting rho2's first use as the first private without an earlier initialisation.

[arporter]

Possibly one for @LonelyCat124 (and @sergisiso) to take a look at as I think we can fix it using the new reverse-dependence lookup functionality.

[LonelyCat124]

I can't access the UM code right now (my account expired while I was on leave). I've checked the PSyclone code that does this generation and either there is a major bug in the code that does this in PSyclone, or rho2 must only be written to inside an ifblock.

I'm not quite clear on why this ifblock bit is written as it is - @sergisiso do you know why when we find a write in an if statement we add to firstprivate and then break? I think we'd even do this for

do ...
if (thing) then
  a = 2
end if
...
a = a + 3
...
end do

Feels like in this case we'd want a to be private or shared - making it firstprivate wouldn't make sense - perhaps I'm just not understanding this code?

[MetBenjaminWent]

Thanks for looking, I am also seeing this occur in another file. Both also have existing OMP directives around the section, which I assume have been historically running fine in the UM.
I saw it as well in this file:
https://code.metoffice.gov.uk/trac/um/browser/main/trunk/src/atmosphere/large_scale_precipitation/lsp_taper_ndrop.F90

Sections with vala = (n_drop_tpr..... l422 for example

These sections also have an if block inside an OMP region, so that could be the common factor.

[sergisiso]

The firstprivate is added because the rho2 is initialised inside a conditional:

IF ( k  <   tdims%k_end ) THEN
  rho2 = ...
  ...
ENDIF

We assume that if this condition is not taken the value of rho2 is what it had before the loop, and therefore needs the firstprivate, this worked well with other compilers.

In this case rho2 is not used anywhere else outside that same "if condition", so we probably can be smarter about it.

[sergisiso]

For Aidan's reference, the case there is something like:

DO 
  IF ( k  <   tdims%k_end ) THEN
    rho2 = ...
    ... uses rho2 ...
  ENDIF
   ... does not use rho2 ...
END DO