(closes #3178) Fixes to handle Directives from fparser

[LonelyCat124]

Blocked by stfc/fparser#486

Will need to update PSyclone to use a specific commit of fparser once that is merged. Tests will fail in the mean time.

Main issue right now is this test fails, and I don't know if/how it should be solved:

    code = """subroutine x()
    !$omp firstprivate
    integer, dimension(100) :: i !dir$ aligned
    i = 1
    end subroutine x"""
    reader = FortranReader(ignore_comments=False, ignore_directives=False)
    psyir = reader.psyir_from_source(code)
    routine = psyir.children[0]
    out = routine.debug_string()
    assert """!$omp firstprivate
integer, dimension(100) :: i  !dir$ aligned""" in out

These directives inside the declaration block have to become comments attached to the declarations, so they get bonus added into them, which may not be correct behaviour, but is not straightforward to fix (other than modifying the backend options).

[sergisiso]

Will need to update PSyclone to use a specific commit of fparser once that is merged. Tests will fail in the mean time.

Sorry for the slow review on that one, but you could already update the submodule to point to the HEAD of your fparser branch here, so the tests would pass. In fact, I wanted to suggest this while looking at the fparser PR, as I was unsure what the best options would be there without looking at what they meant for psyclone.

[LonelyCat124]

Ok, I fixed this to use the submodule now.

[sergisiso]

For some reason this hasn't worked. Getting this branch and doing a "git submodule update" still gives me origin/master

[LonelyCat124]

For some reason this hasn't worked. Getting this branch and doing a "git submodule update" still gives me origin/master

Thats strange, it looks like the pytest on github has correctly used that branch. Maybe what I changed only affects that.

[sergisiso]

Maybe cloning does what the submodules file asks, but for git pulls I think you need to commit the new submodule hash. If I do it manually and do a git diff I get:

git diff
diff --git a/external/fparser b/external/fparser
index 00d276a6d5..7cf48ed76b 160000
--- a/external/fparser
+++ b/external/fparser
@@ -1 +1 @@
-Subproject commit 00d276a6d575d7c79411ba8ace0dc95cd3b505ab
+Subproject commit 7cf48ed76b613346960bc9e428907417c4736492

I think you need to commit this change.

[LonelyCat124]

Should be done now.

[sergisiso]

Just to keep track of some tests I was doing when reviewig the associated fparser PR:

psyclone --keep-comments --keep-directives test.f90
module A
  implicit none
  ! DIR$ ALIAS a,c
  integer, public :: a
  public

  contains
  subroutine test()

    !$ a = 0 +     &
    !$&  0
    !$omp atomic&
    !$omp& update
    a = 1
    !dir$ vector
    a = 2
    !$acc atomic
    a = 3

  end subroutine test

end module A

Statements directives are all preserved, but declaration's are converted to comments.

Without the keep-directives:

psyclone --keep-comments test.f90
module A
  implicit none
  ! DIR$ ALIAS a,c
  integer, public :: a
  public

  contains
  subroutine test()

    a = 1

    ! dir$ vector
    a = 2
    a = 3

  end subroutine test

end module A

This are not technically directives anymore (there is a space) but maybe we should remove them.

[LonelyCat124]

Yeah, I think this was essentially the failing behaviour also from #3196 - I'll need to adapt the regex code from fparser as well to remove the directive comments since at the moment PSyclone is just checking for !$.

[LonelyCat124]

I believe I've fixed that issue now. I will make the directives in/around declarations as an xfailing test.

[sergisiso]

@LonelyCat124 We also need examples with "!$omp end .." specially for the cases that it is at the end of the subroutine and when it is followed by another "!$omp parallel do" loop.

[LonelyCat124]

@sergisiso With the fparser updates, this branch is now working as expected - ready for another look. I've added tests for the directives you asked for (at least as I understood) as well.

[LonelyCat124]

Hmm, somehow test suite isn't running with latest fparser master, I'll check again.

[sergisiso]

@LonelyCat124 Thanks for fixing the issue involving both repositories, this now manages properly most cases of directives inside execution statements.

[sergisiso]

Just to me sure, I submitted the CI once more, as I am not sure if integration test were using the external fparser before, but they should not impact this PR as they have comments and directives disables anyway.