[Mesa-dev] [PATCH 5/5] clover: Dynamically calculate __OPENCL_VERSION__ and CLC language version

[Jan Vesely]

On Tue, 2018-03-20 at 20:23 -0500, Aaron Watry wrote:
> ping.
> 
> This is the last of the series that still needs review.

Hi,

sorry for the delay. I once again really dislike the approach of
implement and enable in separate patches, because it breaks bisection
(I think 4 and 5 should be squashed).

> 
> --Aaron
> 
> On Thu, Mar 1, 2018 at 1:39 PM, Aaron Watry <awatry at gmail.com> wrote:
> > Use get_language_version to calculate default cl standard based on
> > device capabilities and -cl-std specified in build options.
> > 
> > v4: Squash the __OPENCL_VERSION__ and CLC language version patches
> > v3: (Jan) Allow device_version up to 2.2 while device_clc_version
> >     only goes to 2.0
> >     Use get_cl_version to calculate version instead
> > v2: Split out from the previous patch (Pierre)
> > 
> > Signed-off-by: Aaron Watry <awatry at gmail.com>
> > CC: Pierre Moreau <pierre.morrow at free.fr>
> > CC: Jan Vesely <jan.vesely at rutgers.edu>
> > ---
> >  src/gallium/state_trackers/clover/llvm/invocation.cpp | 6 ++++--
> >  1 file changed, 4 insertions(+), 2 deletions(-)
> > 
> > diff --git a/src/gallium/state_trackers/clover/llvm/invocation.cpp b/src/gallium/state_trackers/clover/llvm/invocation.cpp
> > index 8d76f203de..f146695585 100644
> > --- a/src/gallium/state_trackers/clover/llvm/invocation.cpp
> > +++ b/src/gallium/state_trackers/clover/llvm/invocation.cpp
> > @@ -194,7 +194,7 @@ namespace {
> >        compat::set_lang_defaults(c->getInvocation(), c->getLangOpts(),
> >                                  compat::ik_opencl, ::llvm::Triple(target.triple),
> >                                  c->getPreprocessorOpts(),
> > -                                clang::LangStandard::lang_opencl11);
> > +                                get_language_version(opts, device_clc_version));

why is this necessary? clang handles multiple -cl-std= arguments fine
(takes the last on the commandline), so why can't we just prepend -cl-
std=$device_clc_version to the list of arguments? (or set it in
set_lang_defaults if it works the same).

Jan

> > 
> >        c->createDiagnostics(new clang::TextDiagnosticPrinter(
> >                                *new raw_string_ostream(r_log),
> > @@ -225,7 +225,9 @@ namespace {
> >        c.getPreprocessorOpts().Includes.push_back("clc/clc.h");
> > 
> >        // Add definition for the OpenCL version
> > -      c.getPreprocessorOpts().addMacroDef("__OPENCL_VERSION__=110");
> > +      c.getPreprocessorOpts().addMacroDef("__OPENCL_VERSION__=" +
> > +              std::to_string(get_cl_version(
> > +                                  dev.device_version()).version_number));
> > 
> >        // clc.h requires that this macro be defined:
> >        c.getPreprocessorOpts().addMacroDef("cl_clang_storage_class_specifiers");
> > --
> > 2.14.1
> > 

-- 
Jan Vesely <jan.vesely at rutgers.edu>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: This is a digitally signed message part
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20180321/96402f7b/attachment.sig>