naibaf7 / caffe

Caffe: a fast open framework for deep learning. With OpenCL and CUDA support.
http://caffe.berkeleyvision.org/
Other
86 stars 20 forks source link

Two OpenCL programs with no kernels #24

Closed psyhtest closed 8 years ago

psyhtest commented 8 years ago

Running ./build/test/test_all.testbin --gtest_filter=*OpenCLKernelCompileTest* X with the ARM Mali 6.0 driver fails due to the conv_layer_spatial.cl and fft.cl programs containing no kernels. Of course, these programs do contain kernels but they are guarded with some macros that the tests apparently do not define.

This can be easily solved by placing:

__kernel void phony() {}

at the end of each program file.

I believe, however, the intent of the tests is to put all the kernels through the compiler under test. May I suggest that the tests define whatever macros are needed to compile as much of the program source as possible?

Please note that I observed this on the opencl branch of BVLC/caffe. I am reporting it here, however, so it's not lost among hundreds of non-OpenCL issues.

naibaf7 commented 8 years ago

@psyhtest Oh, I wrote this instruction on the other issue you filed without noticing you already got ahead. Sorry.. Note that those two kernel packages are not meant to be used/compiled on your platform. These are Intel kernels provided for Intel CPU and GPUs.

I'll try to add an empty kernel to at least let it compile. Thanks.

psyhtest commented 8 years ago

@naibaf7 No worries! If these programs are not meant to be used on non-Intel platforms, then an empty kernel per program is a good solution. As I understand, all the program files with kernels (*.cl) get concatenated, so the kernel names should be unique.

psyhtest commented 8 years ago

@naibaf7 In fact, given program concatenation and Dtype templating, it's probably better to do something like:

// src/caffe/greentea/cl_kernels/conv_layer_spatial.cl
+__kernel void TEMPLATE(conv_layer_spatial_phony,Dtype)() {}

// src/caffe/greentea/cl_kernels/fft.cl
+__kernel void TEMPLATE(fft_phony,Dtype)() {}

What do you think?

naibaf7 commented 8 years ago

Yes that looks perfect. Do you want to do a pull request or should I just put it in?

naibaf7 commented 8 years ago

Changed / fixed.

psyhtest commented 8 years ago

@naibaf7 Many thanks for the fix! In the future, I'll do pull requests. Do I understand correctly that you prefer them against the master branch of naibaf7/caffe, not the opencl branch of BVLC/caffe?

naibaf7 commented 8 years ago

@psyhtest It does not matter against which branch for me. If other people should be involved in the issue discussion, BVLC/caffe opencl is preferred.

The chance that I notice a pull request is higher on naibaf7/caffe though. naibaf7/caffe might have experimental code not yet ready for BVLC/caffe opencl sometimes, although I keep them pretty close together.