From 838922fb5574aa17e63b07b84c11c957e4a5ea1e Mon Sep 17 00:00:00 2001 From: Marco Antognini Date: Tue, 27 Nov 2018 14:54:58 +0000 Subject: [PATCH] Derive builtin return type from its definition Summary: Prior to this patch, OpenCL code such as the following would attempt to create a BranchInst with a non-bool argument: if (enqueue_kernel(get_default_queue(), 0, nd, ^(void){})) /* ... */ This patch is a follow up on a similar issue with pipe builtin operations. See commit r280800 and https://bugs.llvm.org/show_bug.cgi?id=30219. This change, while being conservative on non-builtin functions, should set the type of expressions invoking builtins to the proper type, instead of defaulting to `bool` and requiring manual overrides in Sema::CheckBuiltinFunctionCall. In addition to tests for enqueue_kernel, the tests are extended to check other OpenCL builtins. Reviewers: Anastasia, spatel, rsmith Reviewed By: Anastasia Subscribers: kristina, cfe-commits, svenvh Differential Revision: https://reviews.llvm.org/D52879 git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@347658 91177308-0d34-0410-b5e6-96231b3b80d8 --- lib/Sema/SemaChecking.cpp | 2 - lib/Sema/SemaExpr.cpp | 20 ++++--- test/CodeGenOpenCL/builtins.cl | 83 ++++++++++++++++++++++++++++++ test/CodeGenOpenCL/pipe_builtin.cl | 22 -------- 4 files changed, 95 insertions(+), 32 deletions(-) create mode 100644 test/CodeGenOpenCL/builtins.cl diff --git a/lib/Sema/SemaChecking.cpp b/lib/Sema/SemaChecking.cpp index a45d4fdfd7..b74be1b30a 100644 --- a/lib/Sema/SemaChecking.cpp +++ b/lib/Sema/SemaChecking.cpp @@ -1308,7 +1308,6 @@ Sema::CheckBuiltinFunctionCall(FunctionDecl *FDecl, unsigned BuiltinID, // check for the argument. if (SemaBuiltinRWPipe(*this, TheCall)) return ExprError(); - TheCall->setType(Context.IntTy); break; case Builtin::BIreserve_read_pipe: case Builtin::BIreserve_write_pipe: @@ -1340,7 +1339,6 @@ Sema::CheckBuiltinFunctionCall(FunctionDecl *FDecl, unsigned BuiltinID, case Builtin::BIget_pipe_max_packets: if (SemaBuiltinPipePackets(*this, TheCall)) return ExprError(); - TheCall->setType(Context.UnsignedIntTy); break; case Builtin::BIto_global: case Builtin::BIto_local: diff --git a/lib/Sema/SemaExpr.cpp b/lib/Sema/SemaExpr.cpp index c9be58be9f..8921d851ff 100644 --- a/lib/Sema/SemaExpr.cpp +++ b/lib/Sema/SemaExpr.cpp @@ -5547,12 +5547,17 @@ Sema::BuildResolvedCallExpr(Expr *Fn, NamedDecl *NDecl, // We special-case function promotion here because we only allow promoting // builtin functions to function pointers in the callee of a call. ExprResult Result; + QualType ReturnTy; if (BuiltinID && Fn->getType()->isSpecificBuiltinType(BuiltinType::BuiltinFn)) { - Result = ImpCastExprToType(Fn, Context.getPointerType(FDecl->getType()), - CK_BuiltinFnToFnPtr).get(); + // Extract the return type from the (builtin) function pointer type. + auto FnPtrTy = Context.getPointerType(FDecl->getType()); + Result = ImpCastExprToType(Fn, FnPtrTy, CK_BuiltinFnToFnPtr).get(); + auto FnTy = FnPtrTy->getPointeeType()->castAs(); + ReturnTy = FnTy->getReturnType(); } else { Result = CallExprUnaryConversions(Fn); + ReturnTy = Context.BoolTy; } if (Result.isInvalid()) return ExprError(); @@ -5562,13 +5567,12 @@ Sema::BuildResolvedCallExpr(Expr *Fn, NamedDecl *NDecl, // of arguments and function on error. CallExpr *TheCall; if (Config) - TheCall = new (Context) CUDAKernelCallExpr(Context, Fn, - cast(Config), Args, - Context.BoolTy, VK_RValue, - RParenLoc); + TheCall = + new (Context) CUDAKernelCallExpr(Context, Fn, cast(Config), + Args, ReturnTy, VK_RValue, RParenLoc); else - TheCall = new (Context) CallExpr(Context, Fn, Args, Context.BoolTy, - VK_RValue, RParenLoc); + TheCall = new (Context) + CallExpr(Context, Fn, Args, ReturnTy, VK_RValue, RParenLoc); if (!getLangOpts().CPlusPlus) { // C cannot always handle TypoExpr nodes in builtin calls and direct diff --git a/test/CodeGenOpenCL/builtins.cl b/test/CodeGenOpenCL/builtins.cl new file mode 100644 index 0000000000..3fba83dcf5 --- /dev/null +++ b/test/CodeGenOpenCL/builtins.cl @@ -0,0 +1,83 @@ +// RUN: %clang_cc1 %s -finclude-default-header -cl-std=CL2.0 -O0 -emit-llvm -o - -triple "spir-unknown-unknown" | FileCheck %s + +void testBranchingOnEnqueueKernel(queue_t default_queue, unsigned flags, ndrange_t ndrange) { + // Ensure `enqueue_kernel` can be branched upon. + + if (enqueue_kernel(default_queue, flags, ndrange, ^(void) {})) + (void)0; + // CHECK: [[P:%[0-9]+]] = call i32 @__enqueue_kernel + // CHECK-NEXT: [[Q:%[a-z0-9]+]] = icmp ne i32 [[P]], 0 + // CHECK-NEXT: br i1 [[Q]] + + if (get_kernel_work_group_size(^(void) {})) + (void)0; + // CHECK: [[P:%[0-9]+]] = call i32 @__get_kernel_work_group_size + // CHECK-NEXT: [[Q:%[a-z0-9]+]] = icmp ne i32 [[P]], 0 + // CHECK-NEXT: br i1 [[Q]] + + if (get_kernel_preferred_work_group_size_multiple(^(void) {})) + (void)0; + // CHECK: [[P:%[0-9]+]] = call i32 @__get_kernel_preferred_work_group_size_multiple_impl + // CHECK-NEXT: [[Q:%[a-z0-9]+]] = icmp ne i32 [[P]], 0 + // CHECK-NEXT: br i1 [[Q]] +} + +void testBranchinOnPipeOperations(read_only pipe int r, write_only pipe int w, global int* ptr) { + // Verify that return type is correctly casted to i1 value. + + if (read_pipe(r, ptr)) + (void)0; + // CHECK: [[R:%[0-9]+]] = call i32 @__read_pipe_2 + // CHECK-NEXT: icmp ne i32 [[R]], 0 + + if (write_pipe(w, ptr)) + (void)0; + // CHECK: [[R:%[0-9]+]] = call i32 @__write_pipe_2 + // CHECK-NEXT: icmp ne i32 [[R]], 0 + + if (get_pipe_num_packets(r)) + (void)0; + // CHECK: [[R:%[0-9]+]] = call i32 @__get_pipe_num_packets_ro + // CHECK-NEXT: icmp ne i32 [[R]], 0 + + if (get_pipe_num_packets(w)) + (void)0; + // CHECK: [[R:%[0-9]+]] = call i32 @__get_pipe_num_packets_wo + // CHECK-NEXT: icmp ne i32 [[R]], 0 + + if (get_pipe_max_packets(r)) + (void)0; + // CHECK: [[R:%[0-9]+]] = call i32 @__get_pipe_max_packets_ro + // CHECK-NEXT: icmp ne i32 [[R]], 0 + + if (get_pipe_max_packets(w)) + (void)0; + // CHECK: [[R:%[0-9]+]] = call i32 @__get_pipe_max_packets_wo + // CHECK-NEXT: icmp ne i32 [[R]], 0 +} + +void testBranchingOnAddressSpaceCast(generic long* ptr) { + // Verify that pointer types are properly casted, respecting address spaces. + + if (to_global(ptr)) + (void)0; + // CHECK: [[P:%[0-9]+]] = call [[GLOBAL_VOID:i8 addrspace\(1\)\*]] @__to_global([[GENERIC_VOID:i8 addrspace\(4\)\*]] {{%[0-9]+}}) + // CHECK-NEXT: [[Q:%[0-9]+]] = bitcast [[GLOBAL_VOID]] [[P]] to [[GLOBAL_i64:i64 addrspace\(1\)\*]] + // CHECK-NEXT: [[BOOL:%[a-z0-9]+]] = icmp ne [[GLOBAL_i64]] [[Q]], null + // CHECK-NEXT: br i1 [[BOOL]] + + if (to_local(ptr)) + (void)0; + // CHECK: [[P:%[0-9]+]] = call [[LOCAL_VOID:i8 addrspace\(3\)\*]] @__to_local([[GENERIC_VOID]] {{%[0-9]+}}) + // CHECK-NEXT: [[Q:%[0-9]+]] = bitcast [[LOCAL_VOID]] [[P]] to [[LOCAL_i64:i64 addrspace\(3\)\*]] + // CHECK-NEXT: [[BOOL:%[a-z0-9]+]] = icmp ne [[LOCAL_i64]] [[Q]], null + // CHECK-NEXT: br i1 [[BOOL]] + + if (to_private(ptr)) + (void)0; + // CHECK: [[P:%[0-9]+]] = call [[PRIVATE_VOID:i8\*]] @__to_private([[GENERIC_VOID]] {{%[0-9]+}}) + // CHECK-NEXT: [[Q:%[0-9]+]] = bitcast [[PRIVATE_VOID]] [[P]] to [[PRIVATE_i64:i64\*]] + // CHECK-NEXT: [[BOOL:%[a-z0-9]+]] = icmp ne [[PRIVATE_i64]] [[Q]], null + // CHECK-NEXT: br i1 [[BOOL]] +} + diff --git a/test/CodeGenOpenCL/pipe_builtin.cl b/test/CodeGenOpenCL/pipe_builtin.cl index d912fce5e9..2a533c54c1 100644 --- a/test/CodeGenOpenCL/pipe_builtin.cl +++ b/test/CodeGenOpenCL/pipe_builtin.cl @@ -69,25 +69,3 @@ void test8(write_only pipe int p, global int *ptr) { // CHECK: call i32 @__get_pipe_max_packets_wo(%opencl.pipe_wo_t* %{{.*}}, i32 4, i32 4) *ptr = get_pipe_max_packets(p); } - -void test9(read_only pipe int r, write_only pipe int w, global int *ptr) { - // verify that return type is correctly casted to i1 value - // CHECK: %[[R:[0-9]+]] = call i32 @__read_pipe_2 - // CHECK: icmp ne i32 %[[R]], 0 - if (read_pipe(r, ptr)) *ptr = -1; - // CHECK: %[[W:[0-9]+]] = call i32 @__write_pipe_2 - // CHECK: icmp ne i32 %[[W]], 0 - if (write_pipe(w, ptr)) *ptr = -1; - // CHECK: %[[NR:[0-9]+]] = call i32 @__get_pipe_num_packets_ro - // CHECK: icmp ne i32 %[[NR]], 0 - if (get_pipe_num_packets(r)) *ptr = -1; - // CHECK: %[[NW:[0-9]+]] = call i32 @__get_pipe_num_packets_wo - // CHECK: icmp ne i32 %[[NW]], 0 - if (get_pipe_num_packets(w)) *ptr = -1; - // CHECK: %[[MR:[0-9]+]] = call i32 @__get_pipe_max_packets_ro - // CHECK: icmp ne i32 %[[MR]], 0 - if (get_pipe_max_packets(r)) *ptr = -1; - // CHECK: %[[MW:[0-9]+]] = call i32 @__get_pipe_max_packets_wo - // CHECK: icmp ne i32 %[[MW]], 0 - if (get_pipe_max_packets(w)) *ptr = -1; -} -- 2.40.0