[Mesa-dev] [PATCH 5/5] clover: Enable cl_khr_fp64 for devices that support doubles v2

Francisco Jerez currojerez at riseup.net
Thu Jul 3 15:28:20 PDT 2014


Tom Stellard <tom at stellard.net> writes:

> On Thu, Jul 03, 2014 at 01:12:07AM +0200, Francisco Jerez wrote:
>> Tom Stellard <tom at stellard.net> writes:
>> 
>> > On Thu, Jun 26, 2014 at 04:15:39PM +0200, Francisco Jerez wrote:
>> >> Tom Stellard <thomas.stellard at amd.com> writes:
>> >> 
>> >> > v2:
>> >> >   - Report correct values for CL_DEVICE_NATIVE_VECTOR_WIDTH_DOUBLE and
>> >> >     CL_DEVICE_PREFERRED_VECTOR_WIDTH_DOUBLE.
>> >> >   - Only define cl_khr_fp64 if the extension is supported.
>> >> >   - Remove trailing space from extension string.
>> >> >   - Rename device query function from cl_khr_fp86() to has_doubles().
>> >> > ---
>> >> >  src/gallium/state_trackers/clover/api/device.cpp      | 6 +++---
>> >> >  src/gallium/state_trackers/clover/core/device.cpp     | 6 ++++++
>> >> >  src/gallium/state_trackers/clover/core/device.hpp     | 1 +
>> >> >  src/gallium/state_trackers/clover/core/program.cpp    | 5 ++++-
>> >> >  src/gallium/state_trackers/clover/llvm/invocation.cpp | 1 -
>> >> >  5 files changed, 14 insertions(+), 5 deletions(-)
>> >> >
>> >> > diff --git a/src/gallium/state_trackers/clover/api/device.cpp b/src/gallium/state_trackers/clover/api/device.cpp
>> >> > index 7006702..1176668 100644
>> >> > --- a/src/gallium/state_trackers/clover/api/device.cpp
>> >> > +++ b/src/gallium/state_trackers/clover/api/device.cpp
>> >> > @@ -145,7 +145,7 @@ clGetDeviceInfo(cl_device_id d_dev, cl_device_info param,
>> >> >        break;
>> >> >  
>> >> >     case CL_DEVICE_PREFERRED_VECTOR_WIDTH_DOUBLE:
>> >> > -      buf.as_scalar<cl_uint>() = 2;
>> >> > +      buf.as_scalar<cl_uint>() = dev.has_doubles() ? 2 : 0;
>> >> >        break;
>> >> >  
>> >> >     case CL_DEVICE_PREFERRED_VECTOR_WIDTH_HALF:
>> >> > @@ -290,7 +290,7 @@ clGetDeviceInfo(cl_device_id d_dev, cl_device_info param,
>> >> >        break;
>> >> >  
>> >> >     case CL_DEVICE_EXTENSIONS:
>> >> > -      buf.as_string() = "";
>> >> > +      buf.as_string() = dev.has_doubles() ? "cl_khr_fp64" : "";
>> >> >        break;
>> >> >  
>> >> >     case CL_DEVICE_PLATFORM:
>> >> > @@ -322,7 +322,7 @@ clGetDeviceInfo(cl_device_id d_dev, cl_device_info param,
>> >> >        break;
>> >> >  
>> >> >     case CL_DEVICE_NATIVE_VECTOR_WIDTH_DOUBLE:
>> >> > -      buf.as_scalar<cl_uint>() = 2;
>> >> > +      buf.as_scalar<cl_uint>() = dev.has_doubles() ? 2 : 0;
>> >> >        break;
>> >> >  
>> >> >     case CL_DEVICE_NATIVE_VECTOR_WIDTH_HALF:
>> >> > diff --git a/src/gallium/state_trackers/clover/core/device.cpp b/src/gallium/state_trackers/clover/core/device.cpp
>> >> > index bc6b761..6bf33e0 100644
>> >> > --- a/src/gallium/state_trackers/clover/core/device.cpp
>> >> > +++ b/src/gallium/state_trackers/clover/core/device.cpp
>> >> > @@ -193,6 +193,12 @@ device::half_fp_config() const {
>> >> >     return CL_FP_DENORM | CL_FP_INF_NAN | CL_FP_ROUND_TO_NEAREST;
>> >> >  }
>> >> >  
>> >> > +bool
>> >> > +device::has_doubles() const {
>> >> > +   return pipe->get_shader_param(pipe, PIPE_SHADER_COMPUTE,
>> >> > +                                 PIPE_SHADER_CAP_DOUBLES);
>> >> > +}
>> >> > +
>> >> >  std::vector<size_t>
>> >> >  device::max_block_size() const {
>> >> >     auto v = get_compute_param<uint64_t>(pipe, PIPE_COMPUTE_CAP_MAX_BLOCK_SIZE);
>> >> > diff --git a/src/gallium/state_trackers/clover/core/device.hpp b/src/gallium/state_trackers/clover/core/device.hpp
>> >> > index 16831ab..025c648 100644
>> >> > --- a/src/gallium/state_trackers/clover/core/device.hpp
>> >> > +++ b/src/gallium/state_trackers/clover/core/device.hpp
>> >> > @@ -66,6 +66,7 @@ namespace clover {
>> >> >        cl_device_fp_config single_fp_config() const;
>> >> >        cl_device_fp_config double_fp_config() const;
>> >> >        cl_device_fp_config half_fp_config() const;
>> >> > +      bool has_doubles() const;
>> >> >  
>> >> >        std::vector<size_t> max_block_size() const;
>> >> >        std::string device_name() const;
>> >> > diff --git a/src/gallium/state_trackers/clover/core/program.cpp b/src/gallium/state_trackers/clover/core/program.cpp
>> >> > index e09c3aa..f65f321 100644
>> >> > --- a/src/gallium/state_trackers/clover/core/program.cpp
>> >> > +++ b/src/gallium/state_trackers/clover/core/program.cpp
>> >> > @@ -95,7 +95,10 @@ program::build_status(const device &dev) const {
>> >> >  
>> >> >  std::string
>> >> >  program::build_opts(const device &dev) const {
>> >> > -   return _opts.count(&dev) ? _opts.find(&dev)->second : "";
>> >> > +   std::string opts = _opts.count(&dev) ? _opts.find(&dev)->second : "";
>> >> > +   if (dev.has_doubles())
>> >> > +      opts.append(" -Dcl_khr_fp64");
>> >> > +   return opts;
>> >> 
>> >> This define belongs in the target-specific part of libclc.  With this
>> >> hunk removed this patch is:
>> >> 
>> >
>> > The declarations for double functions in the libclc headers are wrapped in this
>> > macro, so we need to set it here in order to be able to use them from clover.
>> >
>> 
>> This abuses the ::build_opts() accessor to that end, which is only
>> supposed to return the compiler options that were specified by the user
>> at build time, as required by the CL_PROGRAM_BUILD_OPTIONS build param.
>> 
>
> You are right, I can fix that.
>
>> IMO preprocessor macros defined by the spec belong in the standard
>> library.  We probably need a specialization of libclc's header files for
>> each triple (I hadn't noticed you didn't have one already -- it will
>> probably be useful for other reasons too), as you have target-specific
>> specializations of the LLVM bitcode library.
>> 
>
> What sort of target specific things should go in the headers?
> Currently, all target-specific code goes into the library.
>

Wouldn't such a thing be useful for other device-dependent numeric
constants and defines, and for built-in functions that either expand to
a generic in-line implementation or a more efficient device-specific
implementation (possibly using compiler intrinsics)?  Just like the C
standard library.

> -Tom
>
>
>> Thanks.
>> 
>> > -Tom
>> >
>> >> Reviewed-by: Francisco Jerez <currojerez at riseup.net>
>> >> 
>> >> >  }
>> >> >  
>> >> >  std::string
>> >> > diff --git a/src/gallium/state_trackers/clover/llvm/invocation.cpp b/src/gallium/state_trackers/clover/llvm/invocation.cpp
>> >> > index 5d2efc4..f2b4fd9 100644
>> >> > --- a/src/gallium/state_trackers/clover/llvm/invocation.cpp
>> >> > +++ b/src/gallium/state_trackers/clover/llvm/invocation.cpp
>> >> > @@ -183,7 +183,6 @@ namespace {
>> >> >  
>> >> >        // clc.h requires that this macro be defined:
>> >> >        c.getPreprocessorOpts().addMacroDef("cl_clang_storage_class_specifiers");
>> >> > -      c.getPreprocessorOpts().addMacroDef("cl_khr_fp64");
>> >> >  
>> >> >        c.getLangOpts().NoBuiltin = true;
>> >> >        c.getTargetOpts().Triple = triple;
>> >> > -- 
>> >> > 1.8.1.5
>> >> >
>> >> > _______________________________________________
>> >> > mesa-dev mailing list
>> >> > mesa-dev at lists.freedesktop.org
>> >> > http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>> >
>> >
>> >
>> >
>> >> _______________________________________________
>> >> mesa-dev mailing list
>> >> mesa-dev at lists.freedesktop.org
>> >> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 212 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20140704/f9fe082d/attachment.sig>


More information about the mesa-dev mailing list