[Mesa-dev] [PATCH 5/5] clover: Enable cl_khr_fp64 for devices that support doubles v2
Tom Stellard
tom at stellard.net
Thu Jul 3 16:23:24 PDT 2014
On Fri, Jul 04, 2014 at 12:28:20AM +0200, Francisco Jerez wrote:
> 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.
>
I don't understand why device-dependent constants would go in the libclc
header files. Wouldn't they be private to the library implementation?
Also, libclc does use complier intrinsics, but these are all implemented
in the library and not in the header files. There is a
separate library built for each device.
-Tom
> > -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
More information about the mesa-dev
mailing list