[OpenCL] Fix pipe builtin bugs

[pxli168]

Summary

1. Diag should be output if types are not the same.
2. Should compare using canonical type.
3. Refine the diag to be more clear.

[MatsPetersson]

Obvious typo in title. Pipe, not piep.

[Anastasia]

Tests missing!

[pxli168]

Refine other check using the changed diag.
Change test as well.

[yaxunl]

LGTM. Thanks.

[Anastasia]

test/SemaOpenCL/invalid-pipe-builtin-cl2.0.cl

1

Could you add a test case that fails before your modification here to show that your change improves on it!

[pxli168]

Add test for correct case.

[Anastasia]

LGTM!

include/clang/Basic/DiagnosticSemaKinds.td

def err_opencl_type_can_only_be_used_as_function_parameter : Error <
   "type %0 can only be used as a function parameter in OpenCL">;
 
 // OpenCL v2.0 s6.13.6 -- Builtin Pipe Functions
 def err_opencl_builtin_pipe_first_arg : Error<
   "first argument to %0 must be a pipe type">;
 def err_opencl_builtin_pipe_arg_num : Error<
   "invalid number of arguments to function: %0">;
 def err_opencl_builtin_pipe_invalid_arg : Error<
-  "invalid argument type to function %0 (expecting %1)">;
+  "invalid argument type to function %0 (expecting %1 having %2)">;
 def err_opencl_builtin_pipe_invalid_access_modifier : Error<
   "invalid pipe access modifier (expecting %0)">;
 
 // OpenCL access qualifier
 def err_opencl_invalid_access_qualifier : Error<
   "access qualifier can only be used for pipe and image type">;
 def err_opencl_invalid_read_write : Error<
   "access qualifier %0 can not be used for %1 %select{|earlier than OpenCL2.0 version}2">;

lib/Sema/SemaChecking.cpp

 static bool checkOpenCLPipePacketType(Sema &S, CallExpr *Call, unsigned Idx) {
   const Expr *Arg0 = Call->getArg(0);
   const Expr *ArgIdx = Call->getArg(Idx);
   const PipeType *PipeTy = cast<PipeType>(Arg0->getType());
   const QualType EltTy = PipeTy->getElementType();
   const PointerType *ArgTy = ArgIdx->getType()->getAs<PointerType>();
   // The Idx argument should be a pointer and the type of the pointer and
   // the type of pipe element should also be the same.
-  if (!ArgTy || S.Context.hasSameType(EltTy, ArgTy->getPointeeType())) {
+  if (!ArgTy ||
+      !S.Context.hasSameType(
+          EltTy, ArgTy->getPointeeType()->getCanonicalTypeInternal())) {
     S.Diag(Call->getLocStart(), diag::err_opencl_builtin_pipe_invalid_arg)
         << Call->getDirectCallee() << S.Context.getPointerType(EltTy)
-        << ArgIdx->getSourceRange();
+        << ArgIdx->getType() << ArgIdx->getSourceRange();
     return true;
   }
   return false;
 }
 
 // \brief Performs semantic analysis for the read/write_pipe call.
 // \param S Reference to the semantic analyzer.
 // \param Call A pointer to the builtin call.
case 4: {
     if (checkOpenCLPipeArg(S, Call))
       return true;
     // The call with 4 arguments should be
     // read/write_pipe(pipe T, reserve_id_t, uint, T*).
     // Check reserve_id_t.
     if (!Call->getArg(1)->getType()->isReserveIDT()) {
       S.Diag(Call->getLocStart(), diag::err_opencl_builtin_pipe_invalid_arg)
           << Call->getDirectCallee() << S.Context.OCLReserveIDTy
-          << Call->getArg(1)->getSourceRange();
+          << Call->getArg(1)->getType() << Call->getArg(1)->getSourceRange();
       return true;
     }
 
     // Check the index.
     const Expr *Arg2 = Call->getArg(2);
     if (!Arg2->getType()->isIntegerType() &&
         !Arg2->getType()->isUnsignedIntegerType()) {
       S.Diag(Call->getLocStart(), diag::err_opencl_builtin_pipe_invalid_arg)
           << Call->getDirectCallee() << S.Context.UnsignedIntTy
-          << Arg2->getSourceRange();
+          << Arg2->getType() << Arg2->getSourceRange();
       return true;
     }
 
     // Check packet type T.
     if (checkOpenCLPipePacketType(S, Call, 3))
       return true;
   } break;
   default:
static bool SemaBuiltinReserveRWPipe(Sema &S, CallExpr *Call) {
   if (checkOpenCLPipeArg(S, Call))
     return true;
 
   // Check the reserve size.
   if (!Call->getArg(1)->getType()->isIntegerType() &&
       !Call->getArg(1)->getType()->isUnsignedIntegerType()) {
     S.Diag(Call->getLocStart(), diag::err_opencl_builtin_pipe_invalid_arg)
         << Call->getDirectCallee() << S.Context.UnsignedIntTy
-        << Call->getArg(1)->getSourceRange();
+        << Call->getArg(1)->getType() << Call->getArg(1)->getSourceRange();
     return true;
   }
 
   return false;
 }
 
 // \brief Performs a semantic analysis on {work_group_/sub_group_
 //        /_}commit_{read/write}_pipe
 // \param S Reference to the semantic analyzer.
 // \param Call The call to the builtin function to be analyzed.
 // \return True if a semantic error was found, false otherwise.
 static bool SemaBuiltinCommitRWPipe(Sema &S, CallExpr *Call) {
   if (checkArgCount(S, Call, 2))
     return true;
 
   if (checkOpenCLPipeArg(S, Call))
     return true;
 
   // Check reserve_id_t.
   if (!Call->getArg(1)->getType()->isReserveIDT()) {
     S.Diag(Call->getLocStart(), diag::err_opencl_builtin_pipe_invalid_arg)
         << Call->getDirectCallee() << S.Context.OCLReserveIDTy
-        << Call->getArg(1)->getSourceRange();
+        << Call->getArg(1)->getType() << Call->getArg(1)->getSourceRange();
     return true;
   }
 
   return false;
 }
 
 // \brief Performs a semantic analysis on the call to built-in Pipe
 //        Query Functions.

test/SemaOpenCL/invalid-pipe-builtin-cl2.0.cl

 // RUN: %clang_cc1 %s -verify -pedantic -fsyntax-only -cl-std=CL2.0
 
 void test1(read_only pipe int p, global int* ptr){
   int tmp;
   reserve_id_t rid;
 
   // read/write_pipe
+  read_pipe(p, &tmp);
+  read_pipe(p, ptr);
   read_pipe(tmp, p);    // expected-error {{first argument to 'read_pipe' must be a pipe type}}
   read_pipe(p);   // expected-error {{invalid number of arguments to function: 'read_pipe'}}
-  read_pipe(p, tmp, tmp, ptr);   // expected-error {{invalid argument type to function 'read_pipe' (expecting 'reserve_id_t')}}
-  read_pipe(p, rid, rid, ptr);   // expected-error {{invalid argument type to function 'read_pipe' (expecting 'unsigned int')}}
-  read_pipe(p, tmp);   // expected-error {{invalid argument type to function 'read_pipe' (expecting 'int *')}}
+  read_pipe(p, rid, tmp, ptr);
+  read_pipe(p, tmp, tmp, ptr);   // expected-error {{invalid argument type to function 'read_pipe' (expecting 'reserve_id_t' having 'int')}}
+  read_pipe(p, rid, rid, ptr);   // expected-error {{invalid argument type to function 'read_pipe' (expecting 'unsigned int' having 'reserve_id_t')}}
+  read_pipe(p, tmp);   // expected-error {{invalid argument type to function 'read_pipe' (expecting 'int *' having 'int')}}
   write_pipe(p, ptr);    // expected-error {{invalid pipe access modifier (expecting write_only)}}
   write_pipe(p, rid, tmp, ptr);    // expected-error {{invalid pipe access modifier (expecting write_only)}}
 
   // reserve_read/write_pipe
-  reserve_read_pipe(p, ptr);    // expected-error{{invalid argument type to function 'reserve_read_pipe' (expecting 'unsigned int')}}
+  reserve_read_pipe(p, tmp);
+  reserve_read_pipe(p, ptr);    // expected-error{{invalid argument type to function 'reserve_read_pipe' (expecting 'unsigned int' having '__global int *')}}
   work_group_reserve_read_pipe(tmp, tmp);    // expected-error{{first argument to 'work_group_reserve_read_pipe' must be a pipe type}}
   sub_group_reserve_write_pipe(p, tmp);    // expected-error{{invalid pipe access modifier (expecting write_only)}}
 
   // commit_read/write_pipe
+  commit_read_pipe(p, rid);
   commit_read_pipe(tmp, rid);    // expected-error{{first argument to 'commit_read_pipe' must be a pipe type}}
-  work_group_commit_read_pipe(p, tmp);    // expected-error{{invalid argument type to function 'work_group_commit_read_pipe' (expecting 'reserve_id_t')}}
+  work_group_commit_read_pipe(p, tmp);    // expected-error{{invalid argument type to function 'work_group_commit_read_pipe' (expecting 'reserve_id_t' having 'int')}}
   sub_group_commit_write_pipe(p, tmp);    // expected-error{{invalid pipe access modifier (expecting write_only)}}
 }
 
 void test2(write_only pipe int p, global int* ptr){
   int tmp;
   reserve_id_t rid;
 
   // read/write_pipe
+  write_pipe(p, &tmp);
+  write_pipe(p, ptr);
   write_pipe(tmp, p);    // expected-error {{first argument to 'write_pipe' must be a pipe type}}
   write_pipe(p);   // expected-error {{invalid number of arguments to function: 'write_pipe'}}
-  write_pipe(p, tmp, tmp, ptr);   // expected-error {{invalid argument type to function 'write_pipe' (expecting 'reserve_id_t')}}
-  write_pipe(p, rid, rid, ptr);   // expected-error {{invalid argument type to function 'write_pipe' (expecting 'unsigned int')}}
-  write_pipe(p, tmp);   // expected-error {{invalid argument type to function 'write_pipe' (expecting 'int *')}}
+  write_pipe(p, rid, tmp, ptr);
+  write_pipe(p, tmp, tmp, ptr);   // expected-error {{invalid argument type to function 'write_pipe' (expecting 'reserve_id_t' having 'int')}}
+  write_pipe(p, rid, rid, ptr);   // expected-error {{invalid argument type to function 'write_pipe' (expecting 'unsigned int' having 'reserve_id_t')}}
+  write_pipe(p, tmp);   // expected-error {{invalid argument type to function 'write_pipe' (expecting 'int *' having 'int')}}
   read_pipe(p, ptr);    // expected-error {{invalid pipe access modifier (expecting read_only)}}
   read_pipe(p, rid, tmp, ptr);    // expected-error {{invalid pipe access modifier (expecting read_only)}}
 
   // reserve_read/write_pipe
-  reserve_write_pipe(p, ptr);    // expected-error{{invalid argument type to function 'reserve_write_pipe' (expecting 'unsigned int')}}
+  reserve_write_pipe(p, tmp);
+  reserve_write_pipe(p, ptr);    // expected-error{{invalid argument type to function 'reserve_write_pipe' (expecting 'unsigned int' having '__global int *')}}
   work_group_reserve_write_pipe(tmp, tmp);    // expected-error{{first argument to 'work_group_reserve_write_pipe' must be a pipe type}}
   sub_group_reserve_read_pipe(p, tmp);    // expected-error{{invalid pipe access modifier (expecting read_only)}}
 
   // commit_read/write_pipe
+  commit_write_pipe(p, rid);
   commit_write_pipe(tmp, rid);    // expected-error{{first argument to 'commit_write_pipe' must be a pipe type}}
-  work_group_commit_write_pipe(p, tmp);    // expected-error{{invalid argument type to function 'work_group_commit_write_pipe' (expecting 'reserve_id_t')}}
+  work_group_commit_write_pipe(p, tmp);    // expected-error{{invalid argument type to function 'work_group_commit_write_pipe' (expecting 'reserve_id_t' having 'int')}}
   sub_group_commit_read_pipe(p, tmp);    // expected-error{{invalid pipe access modifier (expecting read_only)}}
 }
 
 void test3(){
   int tmp;
   get_pipe_num_packets(tmp);    // expected-error {{first argument to 'get_pipe_num_packets' must be a pipe type}}
   get_pipe_max_packets(tmp);    // expected-error {{first argument to 'get_pipe_max_packets' must be a pipe type}}
 }