[flang][openmp] Add General Semantic Checks for Allocate Directive

[Rin]

Summary

This patch adds semantic checks for the General Restrictions of the Allocate Directive.

Since the requires directive is not yet implemented in Flang, the restriction
"allocate directives that appear in a target region must specify an allocator clause unless a requires directive with the dynamic_allocators clause is present in the same compilation unit"
will need to be updated at a later time.

A different patch will be made with the Fortran specific restrictions of this directive.

I have used the code from https://reviews.llvm.org/D89395 for the CheckObjectListStructure function.

[Rin]

flang/test/Semantics/omp-allocate03.f90

16

This test is currently failing as it is not throwing any errors, although one is expected.

[Rin]

Fix failing test

[kiranchandramohan]

I) Are you handling only a subset of the restrictions here? I see the following in,

1. General restrictions.

-> allocate directives that appear in a target region must specify an allocator clause unless a requires directive with the dynamic_allocators clause is present in the same compilation unit.

2. Fortran specific restrictions.

-> List items specified in the allocate directive must not have the ALLOCATABLE attribute unless the directive is associated with an allocate statement.
-> List items specified in an allocate directive that is associated with an allocate statement must be variables that are allocated by the allocate statement.
-> Multiple directives can only be associated with an allocate statement if list items are specified on each allocate directive.
-> If a list item has the SAVE attribute, is a common block name, or is declared in the scope of a module, then only predefined memory allocator parameters can be used in the allocator clause.
-> A type parameter inquiry cannot appear in an allocate directive.

Since the requires directive is not there I am assuming that only a subset of the check in the general restrictions can be checked now.
Please provide a clarification saying that only a subset of checks are implemented or implement the above checks.

II) A few files have no newline at end of file.

[Rin]

In D91159#2397842, @kiranchandramohan wrote:

I) Are you handling only a subset of the restrictions here? I see the following in,

1. General restrictions.

-> allocate directives that appear in a target region must specify an allocator clause unless a requires directive with the dynamic_allocators clause is present in the same compilation unit.

2. Fortran specific restrictions.

-> List items specified in the allocate directive must not have the ALLOCATABLE attribute unless the directive is associated with an allocate statement.
-> List items specified in an allocate directive that is associated with an allocate statement must be variables that are allocated by the allocate statement.
-> Multiple directives can only be associated with an allocate statement if list items are specified on each allocate directive.
-> If a list item has the SAVE attribute, is a common block name, or is declared in the scope of a module, then only predefined memory allocator parameters can be used in the allocator clause.
-> A type parameter inquiry cannot appear in an allocate directive.

Since the requires directive is not there I am assuming that only a subset of the check in the general restrictions can be checked now.
Please provide a clarification saying that only a subset of checks are implemented or implement the above checks.

II) A few files have no newline at end of file.

I) So sorry about that. Don't know how I managed to miss all of those. I'll start working on them.
II) My editor seems to be doing that, I'll fix it with the next update.

[Rin]

Add semantic check for allocate directives within target region

[Rin]

Rebase with Parser for Allocate Directive patch

[kiranchandramohan]

flang/include/flang/Parser/parse-tree.h

3812–3813

Can you post another patch without these conflicts?

[Rin]

Get rid of conflicts

[Rin]

Get rid of conflicts

[kiranchandramohan]

First few comments.

flang/lib/Semantics/check-omp-structure.cpp

193

Can you reuse the following function?
void OmpStructureChecker::CheckIsVarPartOfAnotherVar

457–458

Why are all these removed in this patch?

flang/lib/Semantics/resolve-directives.cpp

116–117

Why is this change in OpenACC required?

351–352

Is it possible to do the semantic checks without these two globals?
If not this should be initialized with braces.

flang/test/Semantics/omp-allocate01.f90

25

Check newline in all test files.

[clementval]

flang/lib/Semantics/check-omp-structure.cpp

457–458

This would break any allowance check on these clauses. I don't see the reason to remove them in this patch.

flang/lib/Semantics/resolve-directives.cpp

116–117

Please revert this change. OpenACC part should not be touch for OpenMP related code.

812

I guess you can query the current context instead of using this global variable.

1099

I would remove those two globals and for the inTarget you can query the context stack. There is an example in OpenACC (https://github.com/llvm/llvm-project/blob/d02eac0c000984865dd1ce2474715538f8439470/flang/lib/Semantics/check-acc-structure.cpp#L67)

For the hasAllocator you can probably go through the list here to get the information.

1198

Can you reuse the ResolveOmpName function instead of this one?

[Rin]

flang/lib/Semantics/check-omp-structure.cpp

193

Yes, this function wasn't yet here when I started working on this patch. I will reuse this function instead.

457–458

I think I was getting some errors about redefinition of those checks. I'll look into this and re add them next update.

flang/lib/Semantics/resolve-directives.cpp

116–117

It's not, I will revert this next update.

flang/test/Semantics/omp-allocate01.f90

25

Will do.

flang/test/Semantics/omp-allocate03.f90

16

I notice here I took care of only the DeclarativeAllocate case and not the Executable one. I will change this in the next update.

[Rin]

Remove unnecessary global variables and add variable check and test for ExecutableAllocateDirective case

[Rin]

flang/lib/Semantics/resolve-directives.cpp

1099

Thanks for your example @clementval. It was really helpful.

1198

I don't see how ResolveOmpName checks the Scope similarly to the way ResolveOmpObjectScope does. I'm not sure how I could reuse it. I'll take another look at it though.

[clementval]

flang/lib/Semantics/resolve-directives.cpp

1198

Fine as well if you cannot reuse it. Can you maybe add a comment on the function to tell how it is different then ResolveOmpName?

[kiranchandramohan]

Rebasing on main.

[clementval]

flang/lib/Semantics/resolve-directives.cpp

369

Would be even nicer to have a generic function like bool IsNestedInDirective(const llvm::omg::Directive dir) so it can be reused later for other check. Might even be in the common class For OpenACC and OpenMP.

No need to do it in this revision but just keep the idea in mind.