Page MenuHomePhabricator

[flang] Add parser support for OpenMP allocate clause
ClosedPublic

Authored by Rin on Aug 4 2020, 7:48 AM.

Diff Detail

Event Timeline

Rin created this revision.Aug 4 2020, 7:48 AM
Herald added a project: Restricted Project. · View Herald Transcript
Rin requested review of this revision.Aug 4 2020, 7:48 AM
DavidTruby requested changes to this revision.Aug 4 2020, 8:19 AM

LGTM other than formatting, could you run clang-format over this please? Thanks!

flang/lib/Parser/openmp-parsers.cpp
109

I think this line would get split in 2 by clang-format as it is over 80 characters

This revision now requires changes to proceed.Aug 4 2020, 8:19 AM

Thanks @Rin for this patch. A few minor comments.

flang/include/flang/Parser/parse-tree.h
3500–3502

Are the Omp*Clauses in alphabetic order?

flang/lib/Parser/openmp-parsers.cpp
214

Nit: Extraneous line?

flang/test/Semantics/omp-clause-validity01.f90
36

Nit:extra character (at end of line)?

48

Did a custom allocator test work fine?

kiranchandramohan added a project: Restricted Project.Aug 4 2020, 8:31 AM
Rin updated this revision to Diff 283162.Aug 5 2020, 1:35 AM

[flang][OpenMP]Add parser support for OpenMP Allocate Clause Update
Added a test to check behaviour when handling a custom allocator as well as fixing formatting issues

  1. Updating D85212: Add parser support for OpenMP allocate clause #
  2. Enter a brief description of the changes included in this update.
  3. The first line is used as subject, next lines as comment.
Rin updated this revision to Diff 283170.Aug 5 2020, 2:22 AM
Rin marked 4 inline comments as done and an inline comment as not done.

Update to get rid of formatting issues

lebedev.ri retitled this revision from Add parser support for OpenMP allocate clause to [flang] Add parser support for OpenMP allocate clause.Aug 5 2020, 5:06 AM
kiranchandramohan accepted this revision.Aug 5 2020, 7:25 AM

LGTM. Please wait for one more approval.

flang/test/Semantics/omp-clause-validity01.f90
65

Nit: extra line.
No further revision required for this issue. You change when you submit.

This revision is now accepted and ready to land.Aug 5 2020, 7:38 AM
sscalpone accepted this revision.Aug 5 2020, 8:08 AM
sscalpone added a subscriber: sscalpone.

Do you have a negative test that checks for an out-of-place allocate?

Do you have a negative test that checks for an out-of-place allocate?

out of place as in for a different directive where it is not supported?

out of place as in for a different directive where it is not supported?

Yes.

This revision was landed with ongoing or failed builds.Aug 6 2020, 2:45 AM
This revision was automatically updated to reflect the committed changes.
Rin marked an inline comment as done.Aug 6 2020, 3:06 AM

Do you have a negative test that checks for an out-of-place allocate?

Not yet, but they will be added in the Semantic checks, which I'll work on next.

flang/include/flang/Parser/parse-tree.h
3500–3502

Yeah, you're right, I'll change that