[PATCH] D89562: [flang]Add Parser Support for OpenMP Allocate Directive

[Irina Dobrescu via Phabricator via llvm-commits]

Rin added inline comments.


================
Comment at: flang/lib/Parser/openmp-parsers.cpp:457
+    sourced(construct<OpenMPExecutableAllocate>(verbatim("ALLOCATE"_tok),
+        maybe(parenthesized(Parser<OmpObjectList>{})), Parser<OmpClauseList>{},
+        maybe(Parser<OpenMPDeclarativeAllocate>{}) / endOmpLine,
----------------
Rin wrote:
> sameeranjoshi wrote:
> > Rin wrote:
> > > Rin wrote:
> > > > clementval wrote:
> > > > > sameeranjoshi wrote:
> > > > > > In case of `OpenMPExecutableAllocate` are the clauses mandatory as the standard doesn't seem to have square brackets around them?
> > > > > > Or that's wrongly mentioned in standard.
> > > > > > 
> > > > > > As `OmpClauseList` is a list it can be optional.
> > > > > > ```
> > > > > > !$omp allocate[(list)] clause
> > > > > > [!$omp allocate[(list)] clause
> > > > > > [...]]
> > > > > > ```
> > > > > > 
> > > > > > So a test case like
> > > > > > ```
> > > > > >   !$omp allocate(a, b)
> > > > > >      allocate ( darray(a, b) )
> > > > > > ```
> > > > > > becomes invalid in that case.
> > > > > If there are required clauses they are set in the TableGen file. `OmpClauseList` should be used in preference of a stricter parser. 
> > > > Hmm, I'm not too sure about this one. You're right that there are no square brackets, but then there are restrictions  such as this one: 
> > > > 
> > > > 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.
> > > > 
> > > > Which specify when an allocator clause needs to be present on a directive. So maybe they are still optional? I can make them required if my assumption is wrong.
> > > Oh yeah, I forgot about that, you're right.
> > Hmm the restriction makes me think that the standard there is some ambiguity.
> > I am not sure what to do with such case.
> If the clause is required, I can add it in the TableGen file like Valentin mentioned. I'm not sure what the best course of action would be in this case, so I'm open to suggestions.
I'm really unsure here, because the TableGen refers to both the Declarative and Executable case. So if I make the allocator clause required, then it will be required for both cases. And yeah, the standard is a bit ambiguous, so I'm quite unsure here.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D89562/new/

https://reviews.llvm.org/D89562