FORALL was marked as an obsolescent feature

[pawosm-arm]

I've been trying to build Yambo with the new LLVM's flang. Apart from issues caused by multiline YAMBO_ALLOC macro (see issue #152), I've also encountered one more issue in two files, src/bse/K_multiply_by_V.F and src/bse/K_multiply_by_V_transpose.F:

flang/lib/Lower/Bridge.cpp:4885: not yet implemented: assignment to a whole allocatable inside FORALL

in both cases, the affected code was this:

forall(i_g=1:BS_nT_grps,PAR_IND_T_Haydock%element_1D(i_g)) Vo(i_g)%fragment = cZERO

Note that expecting compiler-side fix may be problematic, as FORALL was marked as an obsolescent feature, see https://flang.llvm.org/docs/FortranFeatureHistory.html#fortran-2018

[sangallidavide]

@pawosm-arm we can easily change

forall(i_g=1:BS_nT_grps,PAR_IND_T_Haydock%element_1D(i_g)) Vo(i_g)%fragment = cZERO

with

do i_g=1,BS_nT_grps
  if(.not.PAR_IND_T_Haydock%element_1D(i_g)) cycle
  Vo(i_g)%fragment = cZERO
enddo                                                                  

Would that be ok?

Did you manage to compile with the YAMBO_ALLOC macro?

[pawosm-arm]

@pawosm-arm we can easily change

forall(i_g=1:BS_nT_grps,PAR_IND_T_Haydock%element_1D(i_g)) Vo(i_g)%fragment = cZERO

with

do i_g=1,BS_nT_grps
  if(.not.PAR_IND_T_Haydock%element_1D(i_g)) cycle
  Vo(i_g)%fragment = cZERO
enddo                                                                  

Would that be ok?

Thanks for the hint. That would help the compiler of course, but is there a plan to introduce it in the code, so Flang users could compile it without patching?

Did you manage to compile with the YAMBO_ALLOC macro?

I've set the FPP variable to make use of gfortran's preprocessor. It may feel like a cheating, but then again, the essential part of the compilation and the codegen is being done by Flang.

[sangallidavide]

Thanks for the hint. That would help the compiler of course, but is there a plan to introduce it in the code, so Flang users could compile it without patching?

Sure, we can code that. Although I checked a bit and a better solution seems to be the use of do concurrent

do concurrent (i_g=1:BS_nT_grps,PAR_IND_T_Haydock%element_1D(i_g))
  Vo(i_g)%fragment = cZERO
enddo

One question. We have many forall in the code. The two you spotted are the ones with a "logical condition".
For others, see for example here:
https://github.com/yambo-code/yambo/blob/master/src/bse/K_inversion_Lo.F#L142

Are all the forall statements giving issues, or just the two mentioned above?

I've set the FPP variable to make use of gfortran's preprocessor. It may feel like a cheating, but then again, the essential part of the compilation and the codegen is being done by Flang.

Well, pre-processing and compilation are done independently in the built of the yambo code. I'd say it is a reasonable solution.

[pawosm-arm]

Thanks for the hint. That would help the compiler of course, but is there a plan to introduce it in the code, so Flang users could compile it without patching?

Sure, we can code that. Although I checked a bit and a better solution seems to be the use of do concurrent

do concurrent (i_g=1:BS_nT_grps,PAR_IND_T_Haydock%element_1D(i_g))
  Vo(i_g)%fragment = cZERO
enddo

As long as the current state of DO CONCURRENT doesn't introduce issues of its own...

One question. We have many forall in the code. The two you spotted are the ones with a "logical condition". For others, see for example here: https://github.com/yambo-code/yambo/blob/master/src/bse/K_inversion_Lo.F#L142

Are all the forall statements giving issues, or just the two mentioned above?

Yes I saw that there is more of it, but Flang compiles the others just fine. The problem is only with the ones containing assignment to a whole allocatable.

I've set the FPP variable to make use of gfortran's preprocessor. It may feel like a cheating, but then again, the essential part of the compilation and the codegen is being done by Flang.

Well, pre-processing and compilation are done independently in the built of the yambo code. I'd say it is a reasonable solution.

That was my thinking too, inspired by the statement in this comment regarding a similar situation: llvm/llvm-project#82579 (comment)

[andrea-ferretti]

Thanks to both for the discussion. Regarding forall, totally fine to fix the two offending ones immediately... moreover, I think it would be a good idea to target getting rid of them all (and for my taste, in favour of explicit do rather than do concurrent, at least at present)

[sangallidavide]

Just one extra comment on "do" vs "do concurrent".
I checked around, for example here:
https://stackoverflow.com/questions/38549666/when-should-i-use-do-concurrent-and-when-openmp

My understanding is that it is better to have a standard do, and then decorate it with OpenMP, OpenACC or whatever when we need to improve performances.

However, as for the two cases above, e.g. simple "variable initialization with if condition", we are likely not going to optimize it with any decoration. Then, I'd say the "do concurrent" is better than the simple "do + if condition" since we give the compiler some extra info on how to optimize the loop by himself.