[PATCH] D74144: [OPENMP50]Add basic support for array-shaping operation.

[Alexey Bataev via Phabricator via cfe-commits]

ABataev marked 6 inline comments as done.
ABataev added inline comments.


================
Comment at: clang/lib/Parse/ParseExpr.cpp:2891
                                           ArgExprs);
     }
+  } else if (!getLangOpts().ObjC && getLangOpts().OpenMP >= 50 &&
----------------
jdoerfert wrote:
> Out of curiosity, why do we prevent Objective C here? I doubt anyone runs it in OpenMP mode but it would be kinda cool.
Just used this to prevent the error messages from Objective C tests when enabled OpenMP by default to test that parsing work correctl in the corner cases. Will remove this check.


================
Comment at: clang/lib/Sema/SemaExpr.cpp:4750
+    return OMPArrayShapingExpr::Create(Context, Context.DependentTy, Base,
+                                       LParenLoc, RParenLoc, Dims, Brackets);
+  if (!BaseTy->isAnyPointerType())
----------------
jdoerfert wrote:
> Does this trigger as well if BaseTy is a pointer type but dependent? I mean, why do we need the first check (`!BaseTy->isAnyPointerType() &&`)?
Not sure this is a real issue. If the type is not a pointer type, but the dependent, just skip all checks. All the required checks will be performed upon the instantiation.


================
Comment at: clang/test/OpenMP/parallel_reduction_messages.c:8
+#pragma omp parallel reduction(+ : incomplete, ([10])p) // expected-error {{a reduction list item with incomplete type 'int []'}} omp45-error {{expected expression}} omp50-error {{expected variable name, array element or array section}}
   ;
 }
----------------
jdoerfert wrote:
> Why is there a 50 error here?
Array shaping is not supported for the reductions


================
Comment at: clang/test/OpenMP/task_ast_print.cpp:153
+#pragma omp task untied mergeable depend(out:argv[:a][1], (arr)[0:],([argc][10])argv) if(task: argc > 0) priority(f)
+  // CHECK-NEXT: #pragma omp task untied mergeable depend(out : argv[:a][1],(arr)[0:],([argc][10])argv) if(task: argc > 0) priority(f)
   a = 2;
----------------
jdoerfert wrote:
> We do not complain that argc is non-constant. Is that on purpose?
Yes, the standard allows the use of the non-constant values as the dimensions.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74144