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

[Stuart Brady via Phabricator via cfe-commits]

stuart requested changes to this revision.
stuart added a comment.
This revision now requires changes to proceed.

Looks good.  Small nit about the test case.



================
Comment at: clang/lib/CodeGen/CodeGenModule.cpp:1500
         StringRef typeNameRef = typeName;
         // Turn "unsigned type" to "utype"
         if (Ty.isCanonical()) {
----------------
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.


================
Comment at: clang/test/CodeGenOpenCL/kernel-arg-info.cl:110
 
+kernel void foo9(signed int si1,  global const signed int* si2) {}
+// CHECK: define{{.*}} spir_kernel void @foo9{{[^!]+}}
----------------
`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.)


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D96161/new/

https://reviews.llvm.org/D96161