jyu2 marked 4 inline comments as not done.
jyu2 added a comment.
================
Comment at: clang/lib/Sema/SemaOpenMP.cpp:5825
+ for (Expr *E : RC->varlists())
+ if (!dyn_cast<DeclRefExpr>(E))
+ ImplicitExprs.emplace_back(E);
----------------
ABataev wrote:
> `isa`. Also, what if this is a `MemberExpr`?
Yes isa. Changed. Thanks.
For reduction MemeberExpr is not allowed, it is only allowed variable name,
array element or array section.
================
Comment at: clang/lib/Sema/SemaOpenMP.cpp:18475-18476
+ !QTy.isTriviallyCopyableType(SemaRef.Context)) {
+ if (NoDiagnose)
+ return true;
SemaRef.Diag(SL, diag::warn_omp_non_trivial_type_mapped) << QTy << SR;
----------------
ABataev wrote:
> It still might be good to emit the warning if the reduction type is mappable.
> Also, need to extend this check in general and avoid emitting it if the user
> defined mapper for the type is provided (in a separate patch, not here).
Thanks. Make sense, in this case, implicit map will be add, so warning should
be emit. Changed.
================
Comment at: clang/lib/Sema/SemaOpenMP.cpp:19435
SemaRef, SimpleExpr, CurComponents, CKind, DSAS->getCurrentDirective(),
- /*NoDiagnose=*/false);
+ /*NoDiagnose=*/NoDiagnose);
if (!BE)
----------------
ABataev wrote:
> You can remove `/*NoDiagnose=*/` here.
Changed. Thanks.
================
Comment at: clang/lib/Sema/SemaOpenMP.cpp:19481-19482
if (VD && DSAS->isThreadPrivate(VD)) {
+ if (NoDiagnose)
+ continue;
DSAStackTy::DSAVarData DVar = DSAS->getTopDSA(VD, /*FromParent=*/false);
----------------
ABataev wrote:
> Hmm, should not we still diagnose this case?
The rule is skip the error as well as skip adding implicit map clause. So that
we don't get regression for old code.
================
Comment at: openmp/libomptarget/test/mapping/reduction_implicit_map.cpp:4
+// amdgcn does not have printf definition
+// XFAIL: amdgcn-amd-amdhsa
+
----------------
ABataev wrote:
> Can we make UNSUPPORTED instead of XFAIL?
changed. Thanks.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D108132/new/
https://reviews.llvm.org/D108132
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
