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

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to