ABataev added inline comments.
================
Comment at: clang/lib/Sema/SemaOpenMP.cpp:2053
+ D, [](OpenMPClauseKind K) -> bool { return K != OMPC_unknown; },
+ Level, /*NotLastprivate=*/true) &&
+ !DSAStack->isLoopControlVariable(D, Level).first) {
----------------
`/*NotLastprivate=*/true` is not needed here. Also, could you move it under
control of the if statement above?
================
Comment at: clang/lib/Sema/SemaOpenMP.cpp:2185-2186
if (DVarPrivate.CKind != OMPC_unknown ||
- (VD && DSAStack->getDefaultDSA() == DSA_none))
+ (VD && (DSAStack->getDefaultDSA() == DSA_none ||
+ DSAStack->getDefaultDSA() == DSA_firstprivate)))
return VD ? VD : cast<VarDecl>(DVarPrivate.PrivateCopy->getDecl());
----------------
Why do you need this check here? `getTopDSA` will return `OMPC_firstprivate`
and one of the previous checks should be fired.
================
Comment at: clang/lib/Sema/SemaOpenMP.cpp:3337
+ if (Stack->getDefaultDSA() == DSA_firstprivate &&
+ VD->getStorageClass() == SC_Static &&
+ (CanonicalVD->getDeclContext()->isNamespace() ||
----------------
1. You need to check that the variable is not a loop control variable here.
2. Seems to me you misinterpret the standard. According to the standard `An
object whose identifier is declared without the storage-class specifier
_Thread_local, and either with external or internal linkage or with the
storage-class specifier static, has static storage duration.`. It means that
non-static globals also must be affected since they have external linkage. So,
you need to use `getStorageDuration()` function here and check that it returns
`SD_Static`.
================
Comment at: clang/lib/Sema/SemaOpenMP.cpp:3338-3339
+ VD->getStorageClass() == SC_Static &&
+ (CanonicalVD->getDeclContext()->isNamespace() ||
+ !VD->isLocalVarDeclOrParm())) {
+ VarsWithInheritedDSA[VD] = E;
----------------
I think just `CanonicalVD->getDeclContext()->isFileContext()` must be enough
here.
================
Comment at: clang/lib/Sema/SemaOpenMP.cpp:3344-3349
+ // Create implicit firstprivate variables as necessary under
+ // default(firstprivate).
+ if (Stack->getDefaultDSA() == DSA_firstprivate) {
+ ImplicitFirstprivate.push_back(E);
+ return;
+ }
----------------
This code must be somewhere later, otherwise, you're skipping a lot of checks
for the target-based directives. This must be checked and processed when we
request implicit data sharing attributes with `Stack->getImplicitDSA(VD,
/*FromParent=*/false);`
================
Comment at: clang/lib/Sema/SemaOpenMP.cpp:12745
}
+
return Res;
----------------
Remove this empty line.
================
Comment at: clang/lib/Sema/SemaOpenMP.cpp:12783-12787
+ if (getLangOpts().OpenMP < 51 && Kind == OMP_DEFAULT_firstprivate) {
+ Diag(StartLoc, diag::err_omp_invalid_dsa)
+ << getOpenMPClauseName(OMPC_firstprivate)
+ << getOpenMPClauseName(OMPC_default) << "5.1";
+ }
----------------
Usually, such checks are implemented in ParseOpenMP.cpp rather than in Sema.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D75591/new/
https://reviews.llvm.org/D75591
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
