ABataev added inline comments.
================
Comment at: lib/AST/DeclOpenMP.cpp:164
+ if (NumClauses) {
+ Clauses = (OMPClause **)C.Allocate(sizeof(OMPClause *) * NumClauses);
+ setClauses(CL);
----------------
lildmh wrote:
> ABataev wrote:
> > lildmh wrote:
> > > ABataev wrote:
> > > > No, bad idea. Use tail allocation for the clauses. Check the
> > > > implementation of `OMPRequiresDecl`
> > > I think it is possible to use TrailingObjects for clause storage when the
> > > number of clauses are known before creating the directive (e.g., for
> > > OMPRequiresDecl and OMPExecutableDirective).
> > >
> > > The reason that I had to create OMPDeclareMapperDecl before parsing map
> > > clauses, is the mapper variable (AA in the example below) needs to be
> > > declared within OMPDeclareMapperDecl, because the following map clauses
> > > will use it.
> > >
> > > ```
> > > #pragma omp declare mapper(struct S AA) map(AA.field1)
> > > ```
> > >
> > > A possible way to get around this is to count the number of map clauses
> > > before hand. But this solution is not trivial since the normal method for
> > > parsing map clauses cannot be used (e.g., it does not know AA when
> > > parsing map(AA.field1)). A customized and complex (because it needs to
> > > handle all possible situations) parsing method needs to be created, just
> > > for counting clause number. I think it's not worthy to do this compared
> > > with allocating map clause space later.
> > >
> > > I checked the code for OMPDeclareReductionDecl that you wrote. It also
> > > has to be created before parsing the combiner and initializer. It does
> > > not have a variable number of clauses though.
> > >
> > > Any suggestions?
> > Instead, you can introduce special DeclContext-based declaration and keep
> > the reference to this declaration inside of the `OMPDeclareMapperDecl`.
> Hi Alexey,
>
> Thanks a lot for your quick response! I don't think I understand your idea.
> Can you establish more on that?
>
> In my current implementation, OMPDeclareMapperDecl is used as the DeclConext
> of the variable AA in the above example, and it already includes the
> reference to AA's declaration.
>
> My problem is, I need to create OMPDeclareMapperDecl before parsing map
> clauses. But before parsing map clauses, I don't know the number of clauses.
> Using TrailingObject requires to know how many clauses there are when
> creating OMPDeclareMapperDecl. So I couldn't use TrailingObject.
>
> My current solution is to create OMPDeclareMapperDecl before parsing map
> clauses, and to create the clause storage after parsing finishes.
What I meant, that you don't need to use `OMPDeclareMapperDecl` for this,
instead you can add another (very simple) special declaration based on
`DeclContext` to use it as the parent declaration for the variable. In the
`OMPDeclareMapperDecl` you can keep the reference to this special declaration.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D56326/new/
https://reviews.llvm.org/D56326
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
