[OpenCL] Fix printing of types with signed prefix in arg info metadata

[Anastasia]

Summary

OpenCL spec doesn't seem to describe the behavior correctly for type printing in GetKernelArgInfo (https://www.khronos.org/registry/OpenCL/specs/3.0-unified/html/OpenCL_API.html#clGetKernelArgInfo):

The type name returned will be the argument type name as it was declared with any whitespace removed. If argument type name is an unsigned scalar type (i.e. unsigned char, unsigned short, unsigned int, unsigned long), uchar, ushort, uint and ulong will be returned.

The example with unsigned doesn't seem to comply with the general statement in the first sentence. It is believed that it actually meant to say that the single word spelling should be used for types i.e.

unsigned type -> utype
signed type -> type

This patch fixed the printing of type with signed prefix in spelling.

[stuart]

Looks good. Small nit about the test case.

clang/lib/CodeGen/CodeGenModule.cpp

1498–1499

It'd make sense to push this comment down one line, above the consume_front("unsigned ") call, as it doesn't apply to the consume_front("signed ") call - or reword it so it covers both substitutions.

clang/test/CodeGenOpenCL/kernel-arg-info.cl

110

signed char would be a better test case, here (although it may be good to test signed int as well).

I believe (but I haven't checked in detail) that for int the canonical naming is either int or unsigned int (i.e. signed int will not occur) whereas for char, the canonical naming is unsigned char, signed char (explicitly stated) or simply char (unstated signedness). (I am basing this on metadata that I have seen, and the understanding that in C, the signedness of char with no explicit signed or unsigned specifier is implementation-defined.)

[Anastasia]

Use char instead of int in the test.

[Anastasia]

clang/lib/CodeGen/CodeGenModule.cpp

1498–1499

This has been addressed in the parent review:
https://reviews.llvm.org/D96151

[stuart]

Looks good, just some suggestions about the test.

clang/test/CodeGenOpenCL/kernel-arg-info.cl

110

Suggest sc1 and sc2 as these are now signed chars, not signed ints.

112–118

Would suggest SCHAR_AS_QUAL and SCHAR_TY now that this is using a signed char.

159–162

... and here.

[Anastasia]

Renamed variables in CHECK statements

[Anastasia]

clang/test/CodeGenOpenCL/kernel-arg-info.cl

112–118

FYI also SCHAR_ARG_NAMES and SCHAR_QUAL

[mantognini]

LGTM, thanks. All the comments seem to be addressed.

[Anastasia]

The clarification to the spec: https://github.com/KhronosGroup/OpenCL-Docs/pull/558

clang/lib/CodeGen/CodeGenModule.cpp

for (unsigned i = 0, e = FD->getNumParams(); i != e; ++i) {
       } else
         accessQuals.push_back(llvm::MDString::get(VMContext, "none"));
 
       // Get argument name.
       argNames.push_back(llvm::MDString::get(VMContext, parm->getName()));
 
       auto getTypeSpelling = [&](QualType Ty) {
         auto typeName = Ty.getUnqualifiedType().getAsString(Policy);
 
         if (Ty.isCanonical()) {
           StringRef typeNameRef = typeName;
           // Turn "unsigned type" to "utype"
           if (typeNameRef.consume_front("unsigned "))
             return std::string("u") + typeNameRef.str();
+          if (typeNameRef.consume_front("signed "))
+            return typeNameRef.str();
         }
 
         return typeName;
       };
 
       if (ty->isPointerType()) {
         QualType pointeeTy = ty->getPointeeType();
 

clang/test/CodeGenOpenCL/kernel-arg-info.cl

 // CHECK: !kernel_arg_addr_space ![[PIPE_AS_QUAL:[0-9]+]]
 // CHECK: !kernel_arg_access_qual ![[PIPE_ACCESS_QUAL:[0-9]+]]
 // CHECK: !kernel_arg_type ![[PIPE_TY:[0-9]+]]
 // CHECK: !kernel_arg_base_type ![[PIPE_BASE_TY:[0-9]+]]
 // CHECK: !kernel_arg_type_qual ![[PIPE_QUAL:[0-9]+]]
 // CHECK-NOT: !kernel_arg_name
 // ARGINFO: !kernel_arg_name ![[PIPE_ARG_NAMES:[0-9]+]]
 
+kernel void foo9(signed char sc1,  global const signed char* sc2) {}
+// CHECK: define{{.*}} spir_kernel void @foo9{{[^!]+}}
+// CHECK: !kernel_arg_addr_space ![[SCHAR_AS_QUAL:[0-9]+]]
+// CHECK: !kernel_arg_access_qual ![[MD42]]
+// CHECK: !kernel_arg_type ![[SCHAR_TY:[0-9]+]]
+// CHECK: !kernel_arg_base_type ![[SCHAR_TY]]
+// CHECK: !kernel_arg_type_qual ![[SCHAR_QUAL:[0-9]+]]
+// CHECK-NOT: !kernel_arg_name
+// ARGINFO: !kernel_arg_name ![[SCHAR_ARG_NAMES:[0-9]+]]
+
 // CHECK: ![[MD11]] = !{i32 1, i32 1, i32 1, i32 1, i32 2, i32 2, i32 1, i32 1, i32 1, i32 1, i32 3, i32 3, i32 3, i32 3, i32 3, i32 3, i32 3, i32 3, i32 0, i32 0, i32 0, i32 0}
 // CHECK: ![[MD12]] = !{!"none", !"none", !"none", !"none", !"none", !"none", !"none", !"none", !"none", !"none", !"none", !"none", !"none", !"none", !"none", !"none", !"none", !"none", !"none", !"none", !"none", !"none"}
 // CHECK: ![[MD13]] = !{!"int*", !"int*", !"int*", !"int*", !"int*", !"int*", !"int*", !"int*", !"int*", !"int*", !"int*", !"int*", !"int*", !"int*", !"int*", !"int*", !"int*", !"int*", !"int", !"int", !"int", !"int"}
 // CHECK: ![[MD14]] = !{!"", !"restrict", !"const", !"restrict const", !"const", !"restrict const", !"const volatile", !"restrict const volatile", !"volatile", !"restrict volatile", !"", !"restrict", !"const", !"restrict const", !"const volatile", !"restrict const volatile", !"volatile", !"restrict volatile", !"", !"", !"", !""}
 // ARGINFO: ![[MD15]] = !{!"globalintp", !"globalintrestrictp", !"globalconstintp", !"globalconstintrestrictp", !"constantintp", !"constantintrestrictp", !"globalconstvolatileintp", !"globalconstvolatileintrestrictp", !"globalvolatileintp", !"globalvolatileintrestrictp", !"localintp", !"localintrestrictp", !"localconstintp", !"localconstintrestrictp", !"localconstvolatileintp", !"localconstvolatileintrestrictp", !"localvolatileintp", !"localvolatileintrestrictp", !"X", !"constint", !"constvolatileint", !"volatileint"}
 // CHECK: ![[MD21]] = !{i32 1, i32 1, i32 1, i32 1}
 // CHECK: ![[MD22]] = !{!"read_only", !"read_only", !"write_only", !"read_write"}
 // CHECK: ![[MD23]] = !{!"image1d_t", !"image2d_t", !"image2d_array_t", !"image1d_t"}

 // CHECK: ![[MD75]] = !{!"", !"", !""}
 // ARGINFO: ![[MD76]] = !{!"ro", !"wo", !"rw"}
 // CHECK: ![[PIPE_AS_QUAL]] = !{i32 1, i32 1, i32 1, i32 1, i32 1}
 // CHECK: ![[PIPE_ACCESS_QUAL]] = !{!"read_only", !"read_only", !"read_only", !"read_only", !"write_only"}
 // CHECK: ![[PIPE_TY]] = !{!"int", !"uchar", !"uchar2", !"uchar", !"uchar"}
 // CHECK: ![[PIPE_BASE_TY]] = !{!"int", !"uchar", !"uchar __attribute__((ext_vector_type(2)))", !"uchar", !"uchar"}
 // CHECK: ![[PIPE_QUAL]] = !{!"pipe", !"pipe", !"pipe", !"pipe", !"pipe"}
 // ARGINFO: ![[PIPE_ARG_NAMES]] = !{!"p1", !"p2", !"p3", !"p4", !"p5"}
+// CHECK: ![[SCHAR_AS_QUAL]] = !{i32 0, i32 1}
+// CHECK: ![[SCHAR_TY]] = !{!"char", !"char*"}
+// CHECK: ![[SCHAR_QUAL]] = !{!"", !"const"}
+// ARGINFO: ![[SCHAR_ARG_NAMES]] = !{!"sc1", !"sc2"}