[PATCH] D75591: [OpenMP] Add firstprivate as a default data-sharing attribute to clang

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.


================
Comment at: clang/lib/Sema/SemaOpenMP.cpp:3434-3435
+      if (Stack->getDefaultDSA() == DSA_firstprivate &&
+          VD->getStorageDuration() == SD_Static &&
+          CanonicalVD->getDeclContext()->isFileContext() &&
           !Stack->isLoopControlVariable(VD).first) {
----------------
atmnpatel wrote:
> ABataev wrote:
> > Hmm, maybe move this check to `getDSA()`? If you do it, you can just modify the check on line 3322 
> > ```
> >       if (DVar.CKind == OMPC_unknown && (Stack->getDefaultDSA() == DSA_none || (Stack->getDefaultDSA() == DSA_firstprivate && !Stack->isLoopControlVariable(VD).first)) &&
> >           isImplicitOrExplicitTaskingRegion(DKind) &&
> >           VarsWithInheritedDSA.count(VD) == 0) {
> >         VarsWithInheritedDSA[VD] = E;
> >         return;
> >       }
> > ```
> Didn't we move it this far down to avoid processing it before all of the target-directive relevant checks?
Yes, but seems to me I was wrong. The standard says "Specifying a variable in a map clause of an enclosed construct may cause an implicit reference to the variable in the enclosing construct. Such implicit references are also subject to the data-sharing attribute rules outlined in this section." If I interpret it correctly, it means that data-sharing attributes must be checked at first. Correct me if I wrong here.


================
Comment at: clang/lib/Sema/SemaOpenMP.cpp:3432
+      // data-sharing attribute clause.
+      VarDecl *CanonicalVD = VD->getCanonicalDecl();
+      if (Stack->getDefaultDSA() == DSA_firstprivate &&
----------------
You don't need to get the canonical decl here, VD already points to the canonical decl.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75591