Page MenuHomePhabricator

[OpenCL] An error shall occur if any scalar operand has greater rank than the type of the vector element
ClosedPublic

Authored by echuraev on May 19 2017, 5:40 AM.

Diff Detail

Event Timeline

echuraev created this revision.May 19 2017, 5:40 AM
Anastasia added inline comments.May 24 2017, 12:02 PM
include/clang/Basic/DiagnosticSemaKinds.td
8307

Since it's OpenCL specific rule, could we rename:

err_scalar_type_rank_greater_than_vector_type -> err_opencl_scalar_type_rank_greater_than_vector_type
lib/Sema/SemaExpr.cpp
8344

Could we initialize this with diag::err_typecheck_vector_not_convertable and then just use default return with empty QualType at the end of the function instead of adding an extra one in lines 8369-8374?

echuraev updated this revision to Diff 100219.May 25 2017, 3:19 AM
echuraev marked 2 inline comments as done.
Anastasia accepted this revision.May 26 2017, 3:56 AM

LGTM! Thanks!

This revision is now accepted and ready to land.May 26 2017, 3:56 AM
echuraev closed this revision.May 26 2017, 6:30 AM

Looks like this causes the build bot to fail on SystemZ:
http://lab.llvm.org:8011/builders/clang-s390x-linux/builds/8741

error: 'error' diagnostics expected but not seen:

File /home/uweigand/sandbox/buildbot/clang-s390x-linux/llvm/tools/clang/test/SemaOpenCL/arithmetic-conversions.cl Line 7: scalar operand type has greater rank than the type of the vector element. ('float2' (vector of 2 'float' values) and 'double')
File /home/uweigand/sandbox/buildbot/clang-s390x-linux/llvm/tools/clang/test/SemaOpenCL/arithmetic-conversions.cl Line 9: scalar operand type has greater rank than the type of the vector element. ('double' and 'float2' (vector of 2 'float' values))

error: 'warning' diagnostics seen but not expected:

File /home/uweigand/sandbox/buildbot/clang-s390x-linux/llvm/tools/clang/test/SemaOpenCL/arithmetic-conversions.cl Line 7: double precision constant requires cl_khr_fp64, casting to single precision
File /home/uweigand/sandbox/buildbot/clang-s390x-linux/llvm/tools/clang/test/SemaOpenCL/arithmetic-conversions.cl Line 9: double precision constant requires cl_khr_fp64, casting to single precision
bader added a comment.May 26 2017, 8:43 AM

This issue probably can be fixed by setting SPIR target architecture or just enabling cl_khr_fp64 extension.
Unfortunately, I can't check it today.
Could you revert Egor's commit, please?

Could you revert Egor's commit, please?

I see Renato Golin has already reverted the commit, thanks ...

Hi all,

I tried to reproduce this problem but I'm not able to do it...
I tried to do it in two different ways:

  1. I tried to build llvm by the following steps:

1.1. Checkout llvm and clang:

svn co https://echuraev@llvm.org/svn/llvm-project/llvm/trunk llvm
svn co https://echuraev@llvm.org/svn/llvm-project/cfe/trunk llvm/tools/clang

1.2. Applied patch by the following line: arc patch D33353
1.3. Build llvm:

mkdir build

cd build

cmake -G "Unix Makefiles" ../llvm

make -j 8

1.4. Run tests: make check-clang
Also, I tried to run make check-all

1.5. As a result: all tests were passed.

  1. I tried to build llvm with ninja.

2.1. This step is the same with 1.1. In the next steps I used commands from the following log: http://lab.llvm.org:8011/builders/clang-s390x-linux/builds/8741
2.2. Go to llvm dir and update svn to the target revision: cd llvm && svn update --non-interactive --no-auth-cache --revision 303986
2.3. Go to clang dif and update svn to the target revision: cd tools/clang/ && svn update --non-interactive --no-auth-cache --revision 303986
2.4. Create extra dir and update svn: mkdir tools/extra && cd tools/extra && svn update --non-interactive --no-auth-cache --revision 303986
2.5. Create compiler-rt dir and update svn: cd ../../../../projects && mkdir compiler-rt && cd compiler-rt && svn update --non-interactive --no-auth-cache --revision 303986
2.6. Generate build ninja files:

cd ../../../
mkdir ninja_build && cd ninja_build
cmake -G Ninja ../llvm -DCMAKE_BUILD_TYPE=Release -DLLVM_ENABLE_ASSERTIONS=True '-DLLVM_LIT_ARGS='"'"'-v'"'"'' -DCMAKE_INSTALL_PREFIX=../stage1.install -DLLVM_ENABLE_ASSERTIONS=ON

2.7. Run build by the following command: ninja
2.8. Run tests: ninja check-all
2.9. And also all tests were passed.

Could you please help me? How can I reproduce this issue?

Thank you in advance!

The problem is that the new test case you added does not contain a -triple argument in the compile command. This means that the compile targets the native architecture on the build system, whatever this is. Since OpenCL support on different architectures may be different (e.g. some may support the cl_khr_fp64 extension by default while others do not), you see the error only on some build architectures.

If you want to see the error, I guess you can force targeting SystemZ by adding "-triple s390x-ibm-linux-gnu" to the compile command.

To actually fix the problem, as mentioned above, add a -triple line that specifies an architecture where the extension is always known to be present. (Or else, I think you can force the extension to be available even if it is not by default on a target.)