[OpenCL2.x] Declare/Define “Other Data Types” with global address space in program scope

Hi,

In OpenCL2.x, I think it is not legal to declare/define “Other Data
Types” ndrange_t / queue_t / … etc, with global address space in
program scope, for example:

test.cl

global ndrange_t nrt;
global queue_t qt;

kernel void test(global int *a) { a[0] = 0; }

Given the OpenCL2.x spec states:
https://www.khronos.org/registry/OpenCL/specs/2.2/html/OpenCL_C.html

In 6.5.1. __global (or global):
“Variables defined at program scope and static variables inside a
function can also be declared in the global address space. They can be
defined with any valid OpenCL C data type except for those in Other
Built-in Data Types. In particular, such program scope variables may
be of any user-defined type, or a pointer to a user-defined type.”

But I use clang-13 to compile the test case, it passes without any
warning or error:

clang-13 --target=amdgcn -cl-std=CL2.0 -S -O3 test.cl

Should we add more checks in: clang/lib/Sema/SemaDecl.cpp

/// Returns true if there hasn't been any invalid type diagnosed.
static bool diagnoseOpenCLTypes(Scope *S, Sema &Se, Declarator &D,
                                DeclContext *DC, QualType R) {
  ...

  // OpenCL v1.2 s6.9.r:
  // The event type cannot be used to declare a program scope variable.
  // OpenCL v2.0 s6.9.q:
  // The clk_event_t and reserve_id_t types cannot be declared in program
  // scope.
  if (NULL == S->getParent()) {
- if (R->isReserveIDT() || R->isClkEventT() || R->isEventT()) {
+ if (R->isReserveIDT() || R->isClkEventT() || R->isEventT() ||
+ R.getUnqualifiedType().getAsString() == "ndrange_t" ||
+ R->isQueueT()) {
      Se.Diag(D.getIdentifierLoc(),
              diag::err_invalid_type_for_program_scope_var)
          << R;
      D.setInvalidType();
      return false;
    }
  }

Thanks,
CY

Hi CY,

Indeed, this seems like a bug. Feel free to submit a bug report or upload the review for your patch straight away.

Thanks,
Anastasia

Hi Anastasia,

Thanks :slight_smile:
Could you help me review the patch: https://reviews.llvm.org/D112110

CY