[PATCH] Clang: Reject OpenCL workgroup size requirement of 0

[Aaron Ballman]

> Index: include/clang/Basic/DiagnosticSemaKinds.td
> ===================================================================
> --- include/clang/Basic/DiagnosticSemaKinds.td (revision 208208)
> +++ include/clang/Basic/DiagnosticSemaKinds.td (working copy)
> @@ -2004,6 +2004,8 @@
>    InGroup<ObjCStringConcatenation>;
>  def note_objc_literal_comparison_isequal : Note<
>    "use 'isEqual:' instead">;
> +def err_attribute_argument_is_zero : Error<
> +  "'%0' attribute must be greater than 0">;

You should remove the quotes from this diagnostic.

>
>  let CategoryName = "Cocoa API Issue" in {
>  def warn_objc_redundant_literal_use : Warning<
> Index: lib/Sema/SemaDeclAttr.cpp
> ===================================================================
> --- lib/Sema/SemaDeclAttr.cpp (revision 208208)
> +++ lib/Sema/SemaDeclAttr.cpp (working copy)
> @@ -2179,9 +2179,16 @@
>  static void handleWorkGroupSize(Sema &S, Decl *D,
>                                  const AttributeList &Attr) {
>    uint32_t WGSize[3];
> -  for (unsigned i = 0; i < 3; ++i)
> -    if (!checkUInt32Argument(S, Attr, Attr.getArgAsExpr(i), WGSize[i], i))
> +  for (unsigned i = 0; i < 3; ++i) {
> +    const Expr *E = Attr.getArgAsExpr(i);
> +    if (!checkUInt32Argument(S, Attr, E, WGSize[i], i))
>        return;
> +    if (WGSize[i] == 0) {
> +      S.Diag(Attr.getLoc(), diag::err_attribute_argument_is_zero)
> +        << Attr.getName()->getName() << E->getSourceRange();

Pass in Attr.getName() directly here (this is why you can remove the
quotes from the diagnostic).

> +      return;
> +    }
> +  }
>
>    WorkGroupAttr *Existing = D->getAttr<WorkGroupAttr>();
>    if (Existing && !(Existing->getXDim() == WGSize[0] &&
>

As Joey points out, it needs a testcase to check the new diagnostic.
With that, and changes above, LGTM!

~Aaron

On Tue, May 13, 2014 at 11:18 AM, Joey Gouly <joey.gouly at arm.com> wrote:
> LGTM too, but please add a test case to invalid-kernel-attrs.cl.
>
> -----Original Message-----
> From: cfe-commits-bounces at cs.uiuc.edu
> [mailto:cfe-commits-bounces at cs.uiuc.edu] On Behalf Of David Tweed
> Sent: 13 May 2014 11:50
> To: 'Pedro Ferreira'; cfe-commits at cs.uiuc.edu
> Subject: RE: [PATCH] Clang: Reject OpenCL workgroup size requirement of 0
>
> The patch LGTM.
> -----Original Message-----
> From: cfe-commits-bounces at cs.uiuc.edu
> [mailto:cfe-commits-bounces at cs.uiuc.edu] On Behalf Of Pedro Ferreira
> Sent: 13 May 2014 10:03
> To: cfe-commits at cs.uiuc.edu
> Subject: [PATCH] Clang: Reject OpenCL workgroup size requirement of 0
>
> OpenCL allows us to define the workgroup size used in a kernel in order
> to further optimise the code, specifically to that size.
> The syntax is
> __attribute__((reqd_work_group_size(X, Y, Z)))
>
> where X, Y and Z are compile-time known integers.
> Currently clang allows any of those to be 0, which would be illegal in
> OpenCL.
>
> The attached patch adds this validation; it's currently a draft and any
> comments would be welcome.
>
> (I reported this on bug 19699)
>
>
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
>
>
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits