[PATCH] D26509: [OpenCL] Fix integer parameters of enqueue_kernel

[Anastasia Stulova via cfe-commits]

Anastasia added inline comments.


================
Comment at: lib/CodeGen/CGBuiltin.cpp:2547
+      llvm::Value *NumEvents =
+          Builder.CreateZExtOrTrunc(EmitScalarExpr(E->getArg(3)), Int32Ty);
       llvm::Value *EventList =
----------------
yaxunl wrote:
> should Int32Ty be SizeTy?
I am not changing the width of the event number argument actually because I think 32 bit width is sufficient.  I just want us to accept objects with the larger width types to be passed in case compiler can prove that they fit in 32 bits. This will allow passing sizeof expression for example (even in 64 bit architectures) but it will create diagnostic only in case the value returned doesn't fit in 32 bits.

See CheckImplicitConversion in SemaChecking.cpp.


================
Comment at: lib/Sema/SemaChecking.cpp:453
 
-    // Forth argument has to be any integer type.
-    if (!Arg3->getType()->isIntegerType()) {
-      S.Diag(TheCall->getArg(3)->getLocStart(),
-             diag::err_opencl_enqueue_kernel_expected_type)
-          << "integer";
-      return true;
-    }
+    // Fourth argument has to be any integer type that fits into 32 bits.
+    checkOpenCLEnqueueIntType(S, Arg3, S.Context.UnsignedIntTy);
----------------
yaxunl wrote:
> This comment does not match what this function does. The function checks if the argument is integer type but does not check if it fits into 32 bit. Also the diagnostic msg says it is not an integer type and does not mention the size requirement.
As per above comment I intended to keep the 32 bit width type for the number of events, but the object passed can have large type as soon as the actual value in it fits in 32 bits (if compiler can deduce that). If the value doesn't fit in 32 bits or the value range is not known at compile time, a diagnostic will be generated.


================
Comment at: test/CodeGenOpenCL/cl20-device-side-enqueue.cl:1
-// RUN: %clang_cc1 %s -cl-std=CL2.0 -ffake-address-space-map -O0 -emit-llvm -o - | FileCheck %s
+// RUN: %clang_cc1 %s -cl-std=CL2.0 -ffake-address-space-map -O0 -emit-llvm -o - -triple "spir-unknown-unknown" | FileCOMMON %s --COMMON-prefix=COMMON --COMMON-prefix=B32
+// RUN: %clang_cc1 %s -cl-std=CL2.0 -ffake-address-space-map -O0 -emit-llvm -o -  -triple "spir64-unknown-unknown" | FileCOMMON %s --COMMON-prefix=COMMON --COMMON-prefix=B64
----------------
yaxunl wrote:
> It seems the command is changed by replacing all CHECK with COMMON.
Indeed, I've done something wrong with the string replacement. Will correct that! Thanks!


https://reviews.llvm.org/D26509