OpenCL and Type Poisoning

Hi everyone.

I'm currently investigating how to implement "type poisoning", required
to disallow using some types in OpenCL kernel parameters. As example, an
OpenCL kernel cannot have a size_t parameter.

Since I cannot check the QualType to detect if parameter type is not
good, Peter Collingbourne suggests me to look for something like the
"#pragma GCC poison" attribute.

I've tried to use that pragma, to poison a type, however, the pragma
poison all identifiers, so

#pragma GCC poison size_t

typedef long size_t; // Here a diagnostic is issued

kernel void foor(size_t err) // I want the diagostic to be issuded here.

I have tried to implement an "openlcl_kernel_poison" pragma, but I've
seen that the pragma construct is too far from what I want to do. Thus,
I've tried to implement the same think with an attribute:

typedef long size_t __attribute__((opencl_kernel_poison));

kernel void foo(size_t err); // The diagnostic is issued here

While handling "opencl_kernel_function" attribute (code from Anton's
patch), I recursively walk the typedef chain looking for declaration
marked with the "opencl_kernel_poision" attribute. This seems working,
however I don't think this to be the right solution.

From my point of view, the "opencl_kernel_poison" attribute can be

generalized to a "type_poison" attribute. It can be applied to any type
declararation, marking the defined type poisoned. It should be handled
like a type attribute, (e.g. "address_space"), and stored inside the
clang::Qualifiers class as an extended qualifiers (There should be a
free bit in clang::Qualifiers::Mask).

Using this approach, detecting whether a type is poisoned is
straighforward.

I don't known if that attribute can be usefull also for aggregated
types:

typedef long size_t __attribute__((type_poison))__;

struct err_1 {
  size_t err_2;
};

The struct err_1 should also be poisoned.

I attach the "opencl_kernel_poison" patch, only as reference. What do
you think about the "type_poison" approach. Is it usefull? Can it be
used also in languages other than OpenCL?

Have a nice day,
speziale.ettore@gmail.com

Hi Speziale,

Hi,

Hi Speziale,

> I attach the "opencl_kernel_poison" patch, only as reference. What do
> you think about the "type_poison" approach. Is it usefull? Can it be
> used also in languages other than OpenCL?

I think you forgot to attach the patch :slight_smile:

Sorry, maybe it's time to go to sleep.

Good evening,
speziale.ettore@gmail.com

kernel-poison.diff (10.6 KB)

Having an attribute on typedefs makes sense to me. It does not make
any sense as a type qualifier; among other things, it's a property of types,
not something that makes a different type.

I don't think the OpenCL poisoning semantics are generic enough to be
worth supporting in the standard compiler.

John.

I agree. For constraints like this, it is best for an opencl compiler to walk the parsed AST and do custom validity checks as an AST walk. There is no need to do this as part of Sema.

-Chris

I have tried to implement an "openlcl_kernel_poison" pragma, but I've
seen that the pragma construct is too far from what I want to do. Thus,
I've tried to implement the same think with an attribute:

typedef long size_t __attribute__((opencl_kernel_poison));

kernel void foo(size_t err); // The diagnostic is issued here

While handling "opencl_kernel_function" attribute (code from Anton's
patch), I recursively walk the typedef chain looking for declaration
marked with the "opencl_kernel_poision" attribute. This seems working,
however I don't think this to be the right solution.

>From my point of view, the "opencl_kernel_poison" attribute can be
generalized to a "type_poison" attribute. It can be applied to any type
declararation, marking the defined type poisoned. It should be handled
like a type attribute, (e.g. "address_space"), and stored inside the
clang::Qualifiers class as an extended qualifiers (There should be a
free bit in clang::Qualifiers::Mask).

Using this approach, detecting whether a type is poisoned is
straighforward.

The concept of a poisoned type is distinct from a kernel poisoned
type. Kernel poisoned types are in fact allowed anywhere in an
OpenCL program except in one specific place, namely parameter types
of __kernel functions. So a general concept of type poisoning does
not seem useful for an OpenCL implementation.

Also, I don't know if it's a good idea to store this attribute as
a qualifier bit. The attribute doesn't affect the semantics of
the program beyond the semantic analysis stage, so it's not really
necessary to allocate a bit for it.

Instead, we can store the attribute on the typedef and use
a generic convenience function for non-canonical types which
walks the chain testing for the presence of a specific attribute.
Maybe something like this (Type member function):

template <typename T> T *getTypedefAttr() {
  Type *T = this;
  while (TypedefType *TT = T->getAs<TypedefType>()) {
    TypedefDecl *TD = TT->getDecl();
    if (T *A = TD->getAttr<T>())
      return A;
    T = TD->getUnderlyingType();
  }
  return 0;
}

Then in the trivial case, we can use
T->getTypedefAttr<OpenCLKernelPoisonAttr>()
to test for the attribute.

I don't known if that attribute can be usefull also for aggregated
types:

typedef long size_t __attribute__((type_poison))__;

struct err_1 {
  size_t err_2;
};

The struct err_1 should also be poisoned.

Perhaps. The OpenCL spec doesn't seem to say anything either way
about this, but it seems reasonable to disallow it for the reasons
given in the spec (and perhaps propose this restriction for a later
version of the spec). Likewise pointers to poisoned types, I think.

Thanks,

Hm? I'm confused here. This behaviour is part of the spec, and if
we claim to support OpenCL we should try to support this behaviour.
If not, where should we draw the line between semantic checks which are
done by Clang and those which are done by a specific implementation?

Thanks,

If we know at declaration time which functions are kernels — we do, right?
I know next to nothing about OpenCL — then I think it's reasonable to do this
in Sema as a check during the declaration of kernels.

My comment about "not worth supporting in the standard compiler" was poorly
phrased. I just meant that the semantics here — certain types are forbidden
in certain positions in the parameter lists of certain kinds of functions —
is not nearly generic enough to try to generalize to a non-OpenCL attribute.

John.

Hi,

Instead, we can store the attribute on the typedef and use
a generic convenience function for non-canonical types which
walks the chain testing for the presence of a specific attribute.
Maybe something like this (Type member function):

template <typename T> T *getTypedefAttr() {
  Type *T = this;
  while (TypedefType *TT = T->getAs<TypedefType>()) {
    TypedefDecl *TD = TT->getDecl();
    if (T *A = TD->getAttr<T>())
      return A;
    T = TD->getUnderlyingType();
  }
  return 0;
}

Then in the trivial case, we can use
T->getTypedefAttr<OpenCLKernelPoisonAttr>()
to test for the attribute.

The Type::getTypedefAttr accessor cannot be implemented in Type.h,
because it needs the complete definition of TypedefDecl.h. Decl.h
includes Type.h, so I've implemented Type::getTypedefAttr out-of-line in
Decl.h. It this right? Or getTypedefAttr should be implemented as a
member function of Decl?

Perhaps. The OpenCL spec doesn't seem to say anything either way
about this, but it seems reasonable to disallow it for the reasons
given in the spec (and perhaps propose this restriction for a later
version of the spec). Likewise pointers to poisoned types, I think.

I agree.

Thanks,

Thank you for your advices,
speziale.ettore@gmail.com

kernel-poison-02-02-2011.diff (12.5 KB)

Hi,

I attach the proposed patch for parsing and validating OpenCL kernel
functions. The patch is composed by:

* "kernel" keyword handling:
  the code comes from lates Anton's pathches. Since I have seen that
  the ARM patch is not yet been merged in the main tree, I have
  extrapolated the portion about the "kernel" keyword, and I have based
  my work on this
* "opencl_kernel_poison" attribute:
  can be attached to a typedef definition to mark the defined type, and
  derived ones, unusable as kernel param types
* kernel function semantic checking:
  kernel function has been checked for correct return value and for
  param types (OpenCL 1.1 Specs, 6.8.k). Checking for half data type
  has not been implemented
* OpenCL header:
  that header should define OpenCL derived types, such as size_t and
  vector types. For now, if defines size_t, ptrdiff_t, [u]intptr_t.
  I have preferred using a custom header instead of stddef and stdint
  to not incure in namespace pollution
* tests:
  I have added some test to check the proposed extension

Notes:
I have found a family of functions (Require*) in clang::Sema
implementing custom checking. I have implemented the checking for the
OpenCLKernelAttr attribute in a similar way.

I hope this patch will be usefull for the clang community. It was for
me.
Best regards,
speziale.ettore@gmail.com

anton-kernel-02-08-2011.diff (14.7 KB)