[PATCH] D127803: Generate the capture for field when the field is used in openmp region with implicit default in the member function.

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.


================
Comment at: clang/lib/Sema/TreeTransform.h:11071-11073
+  if (!getDerived().AlwaysRebuild() && !getSema().getLangOpts().OpenMP &&
+      Base.get() == E->getBase() && QualifierLoc == E->getQualifierLoc() &&
+      Member == E->getMemberDecl() && FoundDecl == E->getFoundDecl() &&
----------------
jyu2 wrote:
> ABataev wrote:
> > jyu2 wrote:
> > > ABataev wrote:
> > > > jyu2 wrote:
> > > > > ABataev wrote:
> > > > > > Why do we need this check here?
> > > > > Without, the field is not getting rebuild during the template instantiation.  That cause the field is not getting captured and implicit firstprivate clause is not getting generate.  The test  is added in default_firstprivate_ast_print.cpp
> > > > > where I add check for it on line 58:
> > > > > // DUMP-NEXT:  -OMPFirstprivateClause
> > > > > // DUMP-NEXT:    -DeclRefExpr {{.*}} 'targetDev'
> > > > This should work without changes. Do we need clause in the template function?
> > > Do you mean, we don't need generate clause for instantiation's template function?
> > > 
> > > apply<32>
> > > without this change the dump like:
> > > 
> > > ```
> > > | | `-CXXMethodDecl 0x1617ab8 <line:38:3, line:50:3> line:38:8 imported used apply 'void ()'
> > > | |   |-TemplateArgument integral 32
> > > | |   `-CompoundStmt 0x161c3a0 <col:16, line:50:3>
> > > | |     `-OMPParallelDirective 0x161c350 <line:39:1, col:43>
> > > | |       |-OMPDefaultClause 0x161bf30 <col:22, col:42>
> > > | |       `-CapturedStmt 0x161c310 <line:40:5, line:42:5>
> > > 
> > > With this change
> > > | | `-CXXMethodDecl 0x110cab8 <line:38:3, line:50:3> line:38:8 imported used apply 'void ()'
> > > | |   |-TemplateArgument integral 32
> > > | |   `-CompoundStmt 0x1111860 <col:16, line:50:3>
> > > | |     `-OMPParallelDirective 0x1111808 <line:39:1, col:43>
> > > | |       |-OMPDefaultClause 0x1110f30 <col:22, col:42>
> > > | |       |-OMPFirstprivateClause 0x11117c8 <<invalid sloc>> <implicit>
> > > | |       | `-DeclRefExpr 0x1111790 <line:41:7> 'int' lvalue OMPCapturedExpr 0x1111320 'targetDev' 'int &'
> > > | |       `-CapturedStmt 0x1111590 <line:40:5, line:42:5>
> > > ```
> > > 
> > It should work without this change correctly. If not, need to adjust sema analysis for such templated functions/fields.
> Okay, I remove that change.  Will deal that later.  
> It seems I need to set AlwaysRebuild to true.  But I don't know how that works at this moment.  Will submit other patch for this.
> 
> Thanks.
> 
No, need to tweak the function that builds implicit clauses to build such clauses for non-templated decls, even in the template context


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127803