lildmh marked an inline comment as done.
lildmh added inline comments.

================
Comment at: lib/AST/DeclOpenMP.cpp:164
+  if (NumClauses) {
+    Clauses = (OMPClause **)C.Allocate(sizeof(OMPClause *) * NumClauses);
+    setClauses(CL);
----------------
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.


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

Reply via email to