Bug 15685 - OpenCL 'char' is not signed

[• ichesnokov]

Summary

Only test case is provided, no patch.
It works fine in current version of LLVM.

[asl]

test/CodeGenOpenCL/15685 - OpenCL 'char' is not signed.patch

1 ↗

(On Diff #45966)

This certainly should be a Sema check, not codegen.

[asl]

test/SemaOpenCL/ocl-char-is-not-signed.cl

1 ↗

(On Diff #45977)

Why it's not OpenCL-specific?

[• ichesnokov]

test/SemaOpenCL/ocl-char-is-not-signed.cl

1 ↗

(On Diff #45977)

OpenCL C standard table 6.1.1:
char A signed two’s complement 8-bit integer.

The test is put in OpenCL folder, leaving possibility to have char unsigned in other platforms.

[asl]

test/SemaOpenCL/ocl-char-is-not-signed.cl

1 ↗

(On Diff #45977)

How the folder would enforce OpenCL mode? Why the test won't fail on unsigned targets?

[• ichesnokov]

test/SemaOpenCL/ocl-char-is-not-signed.cl

1 ↗

(On Diff #45977)

I think SemaOpenCL is the folder where from CL and only CL tests run. On unsigned targets it wouldn't run, it runs only on OpenCL. Please correct me if I am wrong.

[asl]

test/SemaOpenCL/ocl-char-is-not-signed.cl

1 ↗

(On Diff #45977)

You're certainly wrong. For now the test would magically work because of .cl extension which will switch the driver in OpenCL mode. No folder name is involved.

In order to make this test proper you need to check that no targer-specific information would "leak" in OpenCL mode, even if you force unsigned char by default.

Also, please familiarize yourself with testing docs (http://llvm.org/docs/TestingGuide.html#regression-test-structure)

[• ichesnokov]

test/SemaOpenCL/ocl-char-is-not-signed.cl

1 ↗

(On Diff #45977)

Ok, will know. Then there's no problem.
Patch is test/SemaOpenCL/ocl-char-is-not-signed.cl and has .cl extension

[• ichesnokov]

BugZilla page: https://llvm.org/bugs/show_bug.cgi?id=15685

[• ichesnokov]

Please review and commit.

[• ichesnokov]

Please review and approve.

[• ichesnokov]

Patch improvements

[• ichesnokov]

Please review and accept.

[Anastasia]

I am missing to understand the intention of this change. Are you trying to improve the testing for OpenCL?

[• ichesnokov]

No, just make test case more 'standard'.

[• ichesnokov]

Instead of checking by division by zero, I've used _StaticAssert. Functionally bo change, but more understandable.

test/SemaOpenCL/char-must-be-signed.cl

+// RUN: %clang_cc1 %s
+
+_Static_assert(sizeof(char) == 1, "In OpenCL char has to be 1 bytes length");
+_Static_assert((char)(0xFFu) < 0, "In OpenCL char type has to be signed.");