Re: [Mesa-dev] [PATCH] clover: Allow OpenCL version override
On 10/13/2016 07:45 PM, Vedran Miletić wrote:
> On 10/07/2016 12:05 PM, Serge Martin wrote:
>> On Thursday 06 October 2016 16:26:21 Vedran Miletić wrote:
>>> PACKAGE_VERSION #ifdef MESA_GIT_SHA1
>>> " (" MESA_GIT_SHA1 ")"
>>> #endif
>>> diff --git a/src/gallium/state_trackers/clover/core/device.cpp
>>> b/src/gallium/state_trackers/clover/core/device.cpp index 8825f99..fce6fb3
>>> 100644
>>> --- a/src/gallium/state_trackers/clover/core/device.cpp
>>> +++ b/src/gallium/state_trackers/clover/core/device.cpp
>>> @@ -24,6 +24,7 @@
>>> #include "core/platform.hpp"
>>> #include "pipe/p_screen.h"
>>> #include "pipe/p_state.h"
>>> +#include "util/u_debug.h"
>>>
>>> using namespace clover;
>>>
>>> @@ -48,6 +49,14 @@ device::device(clover::platform ,
>>> pipe_loader_device *ldev) : pipe->destroy(pipe);
>>>throw error(CL_INVALID_DEVICE);
>>> }
>>> +
>>> + const std::string cl_version_override =
>>> + debug_get_option("CLOVER_CL_VERSION_OVERRIDE",
>>> ""); + ocl_version = !cl_version_override.empty() ? cl_version_override :
>>> "1.1";
>>
>> This is what the default value of debug_get_option is for, this is
>> redundant.
>> You just have to pass "1.1".
>>
>> Also, if we have a util function that statistically keep the value the extra
>> device variable and funcs are not needed.
>> We may need it if we go for CL 2.0 latter with some devices only advertising
>> CL 1.2 because of unsupported feature, but not for the moment.
>>
>
> Thanks for the default value comment, did not know it. Will fix it. I do
> agree with a util function that would return "1.1" until we choose to
> bump it. However I would prefer to have function per-device because we
> will eventually need to do it like that anyway.
>
On a second thought, I believe you are right on this one. I will simplify.
Regards,
Vedran
--
Vedran Miletić
vedran.miletic.net
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] clover: Allow OpenCL version override
On 10/07/2016 12:05 PM, Serge Martin wrote:
> On Thursday 06 October 2016 16:26:21 Vedran Miletić wrote:
>> CLOVER_CL_VERSION_OVERRIDE allows overriding default OpenCL version
>> supported by Clover, analogous to MESA_GL_VERSION_OVERRIDE for OpenGL.
>> CLOVER_CL_C_VERSION_OVERRIDE allows overridng default OpenCL C version.
>
> Thanks.
> I've made some comments bellow
>
>> ---
>> docs/envvars.html | 12
>> src/gallium/state_trackers/clover/api/device.cpp | 4 ++--
>> src/gallium/state_trackers/clover/api/platform.cpp| 4 ++--
>> src/gallium/state_trackers/clover/core/device.cpp | 19
>> +++ src/gallium/state_trackers/clover/core/device.hpp |
>> 4
>> src/gallium/state_trackers/clover/core/platform.cpp | 9 +
>> src/gallium/state_trackers/clover/core/platform.hpp | 3 +++
>> src/gallium/state_trackers/clover/core/program.cpp| 4 +++-
>> src/gallium/state_trackers/clover/llvm/invocation.cpp | 18
>> ++ src/gallium/state_trackers/clover/llvm/invocation.hpp |
>> 1 +
>> src/gallium/state_trackers/clover/llvm/util.hpp | 4 ++--
>> 11 files changed, 71 insertions(+), 11 deletions(-)
>>
>> diff --git a/docs/envvars.html b/docs/envvars.html
>> index cf57ca5..f76827b 100644
>> --- a/docs/envvars.html
>> +++ b/docs/envvars.html
>> @@ -235,6 +235,18 @@ Setting to "tgsi", for example, will print all the TGSI
>> shaders. See src/mesa/state_tracker/st_debug.c for other options.
>>
>>
>> +Clover state tracker environment variables
>> +
>> +
>> +CLOVER_CL_VERSION_OVERRIDE - allows overriding OpenCL version returned
>> by +clGetPlatformInfo(CL_PLATFORM_VERSION) and
>> +clGetDeviceInfo(CL_DEVICE_VERSION). Note that the setting sets the
>> version +of the platform and all the devices to the same value.
>> +CLOVER_CL_C_VERSION_OVERRIDE - allows overriding OpenCL C version
>> reported +by clGetDeviceInfo(CL_DEVICE_OPENCL_C_VERSION) and the value
>> of the +__OPENCL_VERSION__ macro in the OpenCL compiler.
>
> I don't think we need CLOVER_CL_C_VERSION_OVERRIDE.
> CLOVER_CL_VERSION_OVERRIDE
> should be enough.
>
I believe we do because with OpenCL 2.1+ the version of OpenCL C is
still 2.0.
>> +
>> +
>> Softpipe driver environment variables
>>
>> SOFTPIPE_DUMP_FS - if set, the softpipe driver will print fragment
>> shaders diff --git a/src/gallium/state_trackers/clover/api/device.cpp
>> b/src/gallium/state_trackers/clover/api/device.cpp index f7bd61b..e23de7a
>> 100644
>> --- a/src/gallium/state_trackers/clover/api/device.cpp
>> +++ b/src/gallium/state_trackers/clover/api/device.cpp
>> @@ -301,7 +301,7 @@ clGetDeviceInfo(cl_device_id d_dev, cl_device_info
>> param, break;
>>
>> case CL_DEVICE_VERSION:
>> - buf.as_string() = "OpenCL 1.1 Mesa " PACKAGE_VERSION
>> + buf.as_string() = "OpenCL " + dev.opencl_version() + " Mesa "
>> PACKAGE_VERSION #ifdef MESA_GIT_SHA1
>> " (" MESA_GIT_SHA1 ")"
>> #endif
>> @@ -355,7 +355,7 @@ clGetDeviceInfo(cl_device_id d_dev, cl_device_info
>> param, break;
>>
>> case CL_DEVICE_OPENCL_C_VERSION:
>> - buf.as_string() = "OpenCL C 1.1 ";
>> + buf.as_string() = "OpenCL C " + dev.opencl_c_version() + " ";
>>break;
>>
>> case CL_DEVICE_PARENT_DEVICE:
>> diff --git a/src/gallium/state_trackers/clover/api/platform.cpp
>> b/src/gallium/state_trackers/clover/api/platform.cpp index b1b1fdf..f344ec8
>> 100644
>> --- a/src/gallium/state_trackers/clover/api/platform.cpp
>> +++ b/src/gallium/state_trackers/clover/api/platform.cpp
>> @@ -50,7 +50,7 @@ clover::GetPlatformInfo(cl_platform_id d_platform,
>> cl_platform_info param, size_t size, void *r_buf, size_t *r_size) try {
>> property_buffer buf { r_buf, size, r_size };
>>
>> - obj(d_platform);
>> + auto = obj(d_platform);
>>
>> switch (param) {
>> case CL_PLATFORM_PROFILE:
>> @@ -58,7 +58,7 @@ clover::GetPlatformInfo(cl_platform_id d_platform,
>> cl_platform_info param, break;
>>
>> case CL_PLATFORM_VERSION:
>> - buf.as_string() = "OpenCL 1.1 Mesa " PACKAGE_VERSION
>> + buf.as_string() = "OpenCL " + platform.opencl_version() + " Mesa "
>
>
> you don't need to add an opencl_version() func to core/plateform,
> it won't be used except here.
> I prefer if we have an util function that would be used here, in device.cpp
> and invocation.cpp because for the moment this is still a global value.
>
>
>> PACKAGE_VERSION #ifdef MESA_GIT_SHA1
>> " (" MESA_GIT_SHA1 ")"
>> #endif
>> diff --git a/src/gallium/state_trackers/clover/core/device.cpp
>> b/src/gallium/state_trackers/clover/core/device.cpp index 8825f99..fce6fb3
>> 100644
>> --- a/src/gallium/state_trackers/clover/core/device.cpp
>> +++ b/src/gallium/state_trackers/clover/core/device.cpp
>> @@ -24,6 +24,7 @@
>> #include "core/platform.hpp"
>> #include "pipe/p_screen.h"
>> #include "pipe/p_state.h"
>> +#include "util/u_debug.h"
>>
Re: [Mesa-dev] [PATCH] clover: Allow OpenCL version override
On Thursday 06 October 2016 16:26:21 Vedran Miletić wrote:
> CLOVER_CL_VERSION_OVERRIDE allows overriding default OpenCL version
> supported by Clover, analogous to MESA_GL_VERSION_OVERRIDE for OpenGL.
> CLOVER_CL_C_VERSION_OVERRIDE allows overridng default OpenCL C version.
Thanks.
I've made some comments bellow
> ---
> docs/envvars.html | 12
> src/gallium/state_trackers/clover/api/device.cpp | 4 ++--
> src/gallium/state_trackers/clover/api/platform.cpp| 4 ++--
> src/gallium/state_trackers/clover/core/device.cpp | 19
> +++ src/gallium/state_trackers/clover/core/device.hpp |
> 4
> src/gallium/state_trackers/clover/core/platform.cpp | 9 +
> src/gallium/state_trackers/clover/core/platform.hpp | 3 +++
> src/gallium/state_trackers/clover/core/program.cpp| 4 +++-
> src/gallium/state_trackers/clover/llvm/invocation.cpp | 18
> ++ src/gallium/state_trackers/clover/llvm/invocation.hpp |
> 1 +
> src/gallium/state_trackers/clover/llvm/util.hpp | 4 ++--
> 11 files changed, 71 insertions(+), 11 deletions(-)
>
> diff --git a/docs/envvars.html b/docs/envvars.html
> index cf57ca5..f76827b 100644
> --- a/docs/envvars.html
> +++ b/docs/envvars.html
> @@ -235,6 +235,18 @@ Setting to "tgsi", for example, will print all the TGSI
> shaders. See src/mesa/state_tracker/st_debug.c for other options.
>
>
> +Clover state tracker environment variables
> +
> +
> +CLOVER_CL_VERSION_OVERRIDE - allows overriding OpenCL version returned
> by +clGetPlatformInfo(CL_PLATFORM_VERSION) and
> +clGetDeviceInfo(CL_DEVICE_VERSION). Note that the setting sets the
> version +of the platform and all the devices to the same value.
> +CLOVER_CL_C_VERSION_OVERRIDE - allows overriding OpenCL C version
> reported +by clGetDeviceInfo(CL_DEVICE_OPENCL_C_VERSION) and the value
> of the +__OPENCL_VERSION__ macro in the OpenCL compiler.
I don't think we need CLOVER_CL_C_VERSION_OVERRIDE. CLOVER_CL_VERSION_OVERRIDE
should be enough.
> +
> +
> Softpipe driver environment variables
>
> SOFTPIPE_DUMP_FS - if set, the softpipe driver will print fragment
> shaders diff --git a/src/gallium/state_trackers/clover/api/device.cpp
> b/src/gallium/state_trackers/clover/api/device.cpp index f7bd61b..e23de7a
> 100644
> --- a/src/gallium/state_trackers/clover/api/device.cpp
> +++ b/src/gallium/state_trackers/clover/api/device.cpp
> @@ -301,7 +301,7 @@ clGetDeviceInfo(cl_device_id d_dev, cl_device_info
> param, break;
>
> case CL_DEVICE_VERSION:
> - buf.as_string() = "OpenCL 1.1 Mesa " PACKAGE_VERSION
> + buf.as_string() = "OpenCL " + dev.opencl_version() + " Mesa "
> PACKAGE_VERSION #ifdef MESA_GIT_SHA1
> " (" MESA_GIT_SHA1 ")"
> #endif
> @@ -355,7 +355,7 @@ clGetDeviceInfo(cl_device_id d_dev, cl_device_info
> param, break;
>
> case CL_DEVICE_OPENCL_C_VERSION:
> - buf.as_string() = "OpenCL C 1.1 ";
> + buf.as_string() = "OpenCL C " + dev.opencl_c_version() + " ";
>break;
>
> case CL_DEVICE_PARENT_DEVICE:
> diff --git a/src/gallium/state_trackers/clover/api/platform.cpp
> b/src/gallium/state_trackers/clover/api/platform.cpp index b1b1fdf..f344ec8
> 100644
> --- a/src/gallium/state_trackers/clover/api/platform.cpp
> +++ b/src/gallium/state_trackers/clover/api/platform.cpp
> @@ -50,7 +50,7 @@ clover::GetPlatformInfo(cl_platform_id d_platform,
> cl_platform_info param, size_t size, void *r_buf, size_t *r_size) try {
> property_buffer buf { r_buf, size, r_size };
>
> - obj(d_platform);
> + auto = obj(d_platform);
>
> switch (param) {
> case CL_PLATFORM_PROFILE:
> @@ -58,7 +58,7 @@ clover::GetPlatformInfo(cl_platform_id d_platform,
> cl_platform_info param, break;
>
> case CL_PLATFORM_VERSION:
> - buf.as_string() = "OpenCL 1.1 Mesa " PACKAGE_VERSION
> + buf.as_string() = "OpenCL " + platform.opencl_version() + " Mesa "
you don't need to add an opencl_version() func to core/plateform,
it won't be used except here.
I prefer if we have an util function that would be used here, in device.cpp
and invocation.cpp because for the moment this is still a global value.
> PACKAGE_VERSION #ifdef MESA_GIT_SHA1
> " (" MESA_GIT_SHA1 ")"
> #endif
> diff --git a/src/gallium/state_trackers/clover/core/device.cpp
> b/src/gallium/state_trackers/clover/core/device.cpp index 8825f99..fce6fb3
> 100644
> --- a/src/gallium/state_trackers/clover/core/device.cpp
> +++ b/src/gallium/state_trackers/clover/core/device.cpp
> @@ -24,6 +24,7 @@
> #include "core/platform.hpp"
> #include "pipe/p_screen.h"
> #include "pipe/p_state.h"
> +#include "util/u_debug.h"
>
> using namespace clover;
>
> @@ -48,6 +49,14 @@ device::device(clover::platform ,
> pipe_loader_device *ldev) : pipe->destroy(pipe);
>throw error(CL_INVALID_DEVICE);
> }
> +
> + const std::string cl_version_override =
> +
Re: [Mesa-dev] [PATCH] clover: Allow OpenCL version override
On 10/06/2016 07:15 PM, Jan Vesely wrote: > On Thu, 2016-10-06 at 16:26 +0200, Vedran Miletić wrote: >> CLOVER_CL_VERSION_OVERRIDE allows overriding default OpenCL version >> supported by Clover, analogous to MESA_GL_VERSION_OVERRIDE for >> OpenGL. >> CLOVER_CL_C_VERSION_OVERRIDE allows overridng default OpenCL C >> version. > > WHat's the use of CL_C_VERSION_OVERRIDE? as implemented it only > modifies behaviour of the device API query. The specs say that it's > also the default value of -cl-std used by the compiler. does it makes > sense to add "cl-std=" option if CLOVER_CL_C_VERSION_OVERRIDE is > present? > > Jan > You are right, I will look into it. Thanks, Vedran -- Vedran Miletić vedran.miletic.net ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] clover: Allow OpenCL version override
On Thu, 2016-10-06 at 16:26 +0200, Vedran Miletić wrote:
> CLOVER_CL_VERSION_OVERRIDE allows overriding default OpenCL version
> supported by Clover, analogous to MESA_GL_VERSION_OVERRIDE for
> OpenGL.
> CLOVER_CL_C_VERSION_OVERRIDE allows overridng default OpenCL C
> version.
WHat's the use of CL_C_VERSION_OVERRIDE? as implemented it only
modifies behaviour of the device API query. The specs say that it's
also the default value of -cl-std used by the compiler. does it makes
sense to add "cl-std=" option if CLOVER_CL_C_VERSION_OVERRIDE is
present?
Jan
> ---
> docs/envvars.html | 12
>
> src/gallium/state_trackers/clover/api/device.cpp | 4 ++--
> src/gallium/state_trackers/clover/api/platform.cpp| 4 ++--
> src/gallium/state_trackers/clover/core/device.cpp | 19
> +++
> src/gallium/state_trackers/clover/core/device.hpp | 4
> src/gallium/state_trackers/clover/core/platform.cpp | 9 +
> src/gallium/state_trackers/clover/core/platform.hpp | 3 +++
> src/gallium/state_trackers/clover/core/program.cpp| 4 +++-
> src/gallium/state_trackers/clover/llvm/invocation.cpp | 18
> ++
> src/gallium/state_trackers/clover/llvm/invocation.hpp | 1 +
> src/gallium/state_trackers/clover/llvm/util.hpp | 4 ++--
> 11 files changed, 71 insertions(+), 11 deletions(-)
>
> diff --git a/docs/envvars.html b/docs/envvars.html
> index cf57ca5..f76827b 100644
> --- a/docs/envvars.html
> +++ b/docs/envvars.html
> @@ -235,6 +235,18 @@ Setting to "tgsi", for example, will print all
> the TGSI shaders.
> See src/mesa/state_tracker/st_debug.c for other options.
>
>
> +Clover state tracker environment variables
> +
> +
> +CLOVER_CL_VERSION_OVERRIDE - allows overriding OpenCL version
> returned by
> +clGetPlatformInfo(CL_PLATFORM_VERSION) and
> +clGetDeviceInfo(CL_DEVICE_VERSION). Note that the setting sets
> the version
> +of the platform and all the devices to the same value.
> +CLOVER_CL_C_VERSION_OVERRIDE - allows overriding OpenCL C
> version reported
> +by clGetDeviceInfo(CL_DEVICE_OPENCL_C_VERSION) and the value of
> the
> +__OPENCL_VERSION__ macro in the OpenCL compiler.
> +
> +
> Softpipe driver environment variables
>
> SOFTPIPE_DUMP_FS - if set, the softpipe driver will print
> fragment shaders
> diff --git a/src/gallium/state_trackers/clover/api/device.cpp
> b/src/gallium/state_trackers/clover/api/device.cpp
> index f7bd61b..e23de7a 100644
> --- a/src/gallium/state_trackers/clover/api/device.cpp
> +++ b/src/gallium/state_trackers/clover/api/device.cpp
> @@ -301,7 +301,7 @@ clGetDeviceInfo(cl_device_id d_dev,
> cl_device_info param,
> break;
>
> case CL_DEVICE_VERSION:
> - buf.as_string() = "OpenCL 1.1 Mesa " PACKAGE_VERSION
> + buf.as_string() = "OpenCL " + dev.opencl_version() + " Mesa "
> PACKAGE_VERSION
> #ifdef MESA_GIT_SHA1
> " (" MESA_GIT_SHA1 ")"
> #endif
> @@ -355,7 +355,7 @@ clGetDeviceInfo(cl_device_id d_dev,
> cl_device_info param,
> break;
>
> case CL_DEVICE_OPENCL_C_VERSION:
> - buf.as_string() = "OpenCL C 1.1 ";
> + buf.as_string() = "OpenCL C " + dev.opencl_c_version() + " ";
> break;
>
> case CL_DEVICE_PARENT_DEVICE:
> diff --git a/src/gallium/state_trackers/clover/api/platform.cpp
> b/src/gallium/state_trackers/clover/api/platform.cpp
> index b1b1fdf..f344ec8 100644
> --- a/src/gallium/state_trackers/clover/api/platform.cpp
> +++ b/src/gallium/state_trackers/clover/api/platform.cpp
> @@ -50,7 +50,7 @@ clover::GetPlatformInfo(cl_platform_id d_platform,
> cl_platform_info param,
> size_t size, void *r_buf, size_t *r_size)
> try {
> property_buffer buf { r_buf, size, r_size };
>
> - obj(d_platform);
> + auto = obj(d_platform);
>
> switch (param) {
> case CL_PLATFORM_PROFILE:
> @@ -58,7 +58,7 @@ clover::GetPlatformInfo(cl_platform_id d_platform,
> cl_platform_info param,
> break;
>
> case CL_PLATFORM_VERSION:
> - buf.as_string() = "OpenCL 1.1 Mesa " PACKAGE_VERSION
> + buf.as_string() = "OpenCL " + platform.opencl_version() + "
> Mesa " PACKAGE_VERSION
> #ifdef MESA_GIT_SHA1
> " (" MESA_GIT_SHA1 ")"
> #endif
> diff --git a/src/gallium/state_trackers/clover/core/device.cpp
> b/src/gallium/state_trackers/clover/core/device.cpp
> index 8825f99..fce6fb3 100644
> --- a/src/gallium/state_trackers/clover/core/device.cpp
> +++ b/src/gallium/state_trackers/clover/core/device.cpp
> @@ -24,6 +24,7 @@
> #include "core/platform.hpp"
> #include "pipe/p_screen.h"
> #include "pipe/p_state.h"
> +#include "util/u_debug.h"
>
> using namespace clover;
>
> @@ -48,6 +49,14 @@ device::device(clover::platform ,
> pipe_loader_device *ldev) :
> pipe->destroy(pipe);
> throw error(CL_INVALID_DEVICE);
> }
> +
> + const std::string cl_version_override =
> +
Re: [Mesa-dev] [PATCH] clover: Allow OpenCL version override
On 10/06/2016 04:26 PM, Vedran Miletić wrote: > CLOVER_CL_VERSION_OVERRIDE allows overriding default OpenCL version > supported by Clover, analogous to MESA_GL_VERSION_OVERRIDE for OpenGL. > CLOVER_CL_C_VERSION_OVERRIDE allows overridng default OpenCL C version. > --- > docs/envvars.html | 12 > src/gallium/state_trackers/clover/api/device.cpp | 4 ++-- > src/gallium/state_trackers/clover/api/platform.cpp| 4 ++-- > src/gallium/state_trackers/clover/core/device.cpp | 19 > +++ > src/gallium/state_trackers/clover/core/device.hpp | 4 > src/gallium/state_trackers/clover/core/platform.cpp | 9 + > src/gallium/state_trackers/clover/core/platform.hpp | 3 +++ > src/gallium/state_trackers/clover/core/program.cpp| 4 +++- > src/gallium/state_trackers/clover/llvm/invocation.cpp | 18 ++ > src/gallium/state_trackers/clover/llvm/invocation.hpp | 1 + > src/gallium/state_trackers/clover/llvm/util.hpp | 4 ++-- > 11 files changed, 71 insertions(+), 11 deletions(-) > This will conflict with [1] due to both modifying tokenize in an incompatible way. I would prefer if we can merge [1] first, then I will rebase this one. Regards, Vedran [1] https://lists.freedesktop.org/archives/mesa-dev/2016-September/130001.html -- Vedran Miletić vedran.miletic.net ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
