[PATCH] D72811: [WIP][OPENMP5.0] allow lvalue for motion clause

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.


================
Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:7829
         if (EncounteredME) {
-          const auto *FD = dyn_cast<FieldDecl>(EncounteredME->getMemberDecl());
-          unsigned FieldIndex = FD->getFieldIndex();
-
-          // Update info about the lowest and highest elements for this struct
-          if (!PartialStruct.Base.isValid()) {
-            PartialStruct.LowestElem = {FieldIndex, LB};
-            PartialStruct.HighestElem = {FieldIndex, LB};
-            PartialStruct.Base = BP;
-          } else if (FieldIndex < PartialStruct.LowestElem.first) {
-            PartialStruct.LowestElem = {FieldIndex, LB};
-          } else if (FieldIndex > PartialStruct.HighestElem.first) {
-            PartialStruct.HighestElem = {FieldIndex, LB};
+          // MemberExpr can also be FunctionDecl after we allow all lvalue
+          if (const auto *FD =
----------------
cchen wrote:
> ABataev wrote:
> > Here we should not have any functiondecls. Must be an assert.
> Don't we need to support something like `#pragma omp target map(r.S.foo()[:12])`?
It does not lead to the actual mapping. Better to inform the user about this rather than consuming it silently.


================
Comment at: clang/lib/Sema/SemaOpenMP.cpp:15283
+  explicit LocatorChecker(
+      OMPClauseMappableExprCommon::MappableExprComponentList &CurComponents)
+      : CurComponents(CurComponents), DerefCnt(0) {}
----------------
cchen wrote:
> ABataev wrote:
> > Why do you need components list here?
> Since I need to insert the information in `VisitDeclRefExpr`.
You can do this outside of this class if you need to do it. Still. you have this `GetDRE()` member function. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72811