[PATCH] D75077: [OpenMP5.0] Allow pointer arithmetic in motion/map clause

[Chi Chun Chen via Phabricator via cfe-commits]

cchen marked 4 inline comments as done.
cchen added inline comments.


================
Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:7616
           dyn_cast<OMPArraySectionExpr>(I->getAssociatedExpression());
+      const auto *UO = dyn_cast<UnaryOperator>(I->getAssociatedExpression());
       bool IsPointer =
----------------
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?


================
Comment at: clang/lib/Sema/SemaOpenMP.cpp:15698
+  }
+  bool VisitBinaryOperator(BinaryOperator *BO) {
+    if (SemaRef.getLangOpts().OpenMP < 50) {
----------------
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).


================
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);
----------------
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?


================
Comment at: clang/lib/Sema/SemaOpenMP.cpp:15729
+      Components.emplace_back(CTE, nullptr);
+      this->RelevantExpr = cast<Expr>(CTE);
+    }
----------------
ABataev wrote:
> No need for `this->`
Will fix this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75077