-
Notifications
You must be signed in to change notification settings - Fork 23
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Sanity check for OpenCL Library #65
Conversation
mk/opencl.mk
Outdated
OPENCL_LIB ?= /usr/local/cuda-9.1/lib64 | ||
OPENCL_LIB_PATH ?= /usr/local/cuda/lib64 | ||
|
||
OPENCL_LIB := $(shell test -d $(OPENCL_LIB_PATH); echo $$?) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you validate some critical headers as well?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Which headers do you refer? Something like CL/cl.h
in this case?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Users might specify other OpenCL implementation rather than the one shipped by NVIDIA SDK. Take Beignet for example, it is an implementation dedicated to Intel Graphics and suitable for CI purpose. However, it is hard for current scripts to detect its availability.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would give it a try.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The availability of Beignet should be included in another issue #69.
mk/opencl.mk
Outdated
LDFLAGS += -L$(OPENCL_LIB) -lOpenCL | ||
-I$(OPENCL_LIB_PATH)/../include | ||
|
||
LDFLAGS += -L$(OPENCL_LIB_PATH) -lOpenCL |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Before assigning $(LDFLAGS)
, we shall ensure the presence of OpenCL runtime library in advance.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you mean check the presence of libOpenCL.so
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, exactly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done.
@marktwtn, Be aware of the availbility of Intel OpenCL driver which might be required by Travis-CI. |
Related: #69 |
1423ddf
to
145ac16
Compare
145ac16
to
94ec2c9
Compare
@jserv Should the OpenCL sanity check include the Travis-CI coverage? |
CI changes could be addressed in another pull request. Let's concentrate on OpenCL validation here. |
No description provided.