ABataev added inline comments.
================
Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:9965
"memory order clause '%0' is specified here">;
+def err_omp_non_lvalue_in_map_or_motion_clauses: Error<
+ "expected lvalue with no function call in '#pragma omp target update' and
'#pragma omp target map'"
----------------
I would say that this lvalue must be addressable, no? Also, some function calls
can be handled, probably, those returning rvalues, for example, or constexprs.
================
Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:7616
dyn_cast<OMPArraySectionExpr>(I->getAssociatedExpression());
+ const auto *UO = dyn_cast<UnaryOperator>(I->getAssociatedExpression());
bool IsPointer =
----------------
cchen wrote:
> ABataev wrote:
> > What about binary operator?
> Didn't add check here since I didn't add any binary operator into Component
> in the MapBaseChecker class. Should I add it?
I think so. Otherwise those binary expressions are meaningless. Or treated in
some unexpected way.
================
Comment at: clang/lib/Sema/SemaOpenMP.cpp:15698
+ }
+ bool VisitBinaryOperator(BinaryOperator *BO) {
+ if (SemaRef.getLangOpts().OpenMP < 50) {
----------------
cchen wrote:
> ABataev wrote:
> > Better to discuss the implementation for this in a separate patch, it
> > really requires some additional analysis and extra work.
> Why should we do this in another patch? The only thing this patch does is
> extending for pointer arithmetic (doing analysis on BinOp).
You're doing some checks in this patch using counting of '*' and '[' in this
patch. This is definitely not correct. That's why I suggest to move it in a
separate patch and discus the solution there and do not block all other kinds
of expressions.
================
Comment at: clang/lib/Sema/SemaOpenMP.cpp:15712
+ const std::string RTS = RT->getCanonicalTypeInternal().getAsString();
+ auto CntLayer = [](char c) { return c == '*' || c == '['; };
+ size_t LLayerCnt = std::count_if(LTS.begin(), LTS.end(), CntLayer);
----------------
cchen wrote:
> ABataev wrote:
> > Again, bad idea to count this stuff.
> Will it be better if just check if the subtree is an offset? So that we only
> need to check if it does not have any decorator in type?
What do you mean? Exclude, say, RHS or LHS from the analysis, if it is a
complex expression and you can use another part as a base? That's worth trying,
at least.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D75077/new/
https://reviews.llvm.org/D75077
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
