[PATCH] D112110: [OpenCL] queue_t and ndrange_t can't be defined in program scope.

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added inline comments.


================
Comment at: clang/lib/Sema/SemaDecl.cpp:6844
   // scope.
+  // OpenCL v2.0 s6.5.1:
+  // Variables defined at program scope and static variables inside a function
----------------
To make things concise I would suggest dropping OpenCL v2.0 quote since it is identical to OpenCL v3.0.


================
Comment at: clang/lib/Sema/SemaDecl.cpp:6853
+    if (R->isReserveIDT() || R->isClkEventT() || R->isEventT() ||
+        R.getUnqualifiedType().getAsString() == "ndrange_t" || R->isQueueT()) {
       Se.Diag(NewVD->getLocation(),
----------------
Suggest swapping those as string comparisons are more costly.

However, does it work if there is a typedef of `ndrange_t` to some other name? Although it feels like the same issue might apply to other types diagnosed here...


================
Comment at: clang/test/SemaOpenCL/invalid-ndrange.cl:1
+// RUN: %clang_cc1 %s -verify -pedantic -fsyntax-only -cl-std=CL2.0
+// RUN: %clang_cc1 %s -verify -pedantic -fsyntax-only -cl-std=CL3.0
----------------
I imagine we won't have a lot of functionality to test in this file and `invalid-queue.cl`?  Then could we combine them in 1 file with a more generic name like `invalid_types.cl` or `invalid_enqueue_types.cl`.

This can spare us extra clang invocations...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112110