[Mesa-dev] [PATCH 4/5] clover/llvm: Use -cl-std and device version to select language defaults

[Jan Vesely]

On Sat, 2017-07-22 at 13:20 +0200, Pierre Moreau wrote:
> Hi Aaron,
> 
> On 2017-07-21 — 23:19, Aaron Watry wrote:
> > According to section 5.8.4.5 of the 2.0 spec, the CL C version is chosen by:
> >  1) If you have -cl-std=CL1.1+ use the version specified
> >  2) If not, use the highest 1.x version that the device supports
> 
> According to that same part of the spec, clBuildProgram and clCompileProgram
> should fail if the specified CL C version is strictly greater than the version
> the device supports. You could add a check in `get_language_version()` to
> compare `ver` and `device_version`, and throw a `build_error()` exception if
> `ver > device_version`.

These should also be using CLC version rather than device CL version.
OCL allows OCL 1.0 devices to support CLC 1.1 language features. (it
also requires a split of these values in 3/5)
I'm not sure how realistic it is in clover. It does not look like any
clover supported device won't be able to provide at least OCL 1.0
runtime features. So feel free to ignore (but make a note if you choose
 to do so)

Jan

> 
> I have two more comments further down.
> 
> > Curiously, there is no valid value for -cl-std=CL1.0
> > 
> > Signed-off-by: Aaron Watry <awatry at gmail.com>
> > ---
> >  .../state_trackers/clover/llvm/invocation.cpp      | 48 ++++++++++++++++++++--
> >  1 file changed, 45 insertions(+), 3 deletions(-)
> > 
> > diff --git a/src/gallium/state_trackers/clover/llvm/invocation.cpp b/src/gallium/state_trackers/clover/llvm/invocation.cpp
> > index 364aaf1517..92d72e5b73 100644
> > --- a/src/gallium/state_trackers/clover/llvm/invocation.cpp
> > +++ b/src/gallium/state_trackers/clover/llvm/invocation.cpp
> > @@ -93,6 +93,48 @@ namespace {
> >        return ctx;
> >     }
> >  
> > +   clang::LangStandard::Kind
> > +   get_language_from_version_str(const std::string &version_str,
> > +                                 bool is_opt = false) {
> > +       /**
> > +        * Per CL 2.0 spec, section 5.8.4.5:
> > +        * If it's an option, use the value directly.
> > +        * If it's a device version, clamp to max 1.x version, a.k.a. 1.2
> > +        */
> > +       if (version_str == "1.1")
> > +          return clang::LangStandard::lang_opencl11;
> > +       if (version_str == "1.2")
> > +          return clang::LangStandard::lang_opencl12;
> > +       if (version_str == "2.0"){
> > +          if (is_opt) return clang::LangStandard::lang_opencl20;
> > +          else return clang::LangStandard::lang_opencl12;
> > +       }
> > +
> > +       /*
> > +        * At this point, it's not a recognized language version option or
> > +        * 1.1+ device version, which just leaves 1.0 as a possible device
> > +        * version (or an invalid version string).
> > +        */
> > +       return clang::LangStandard::lang_opencl10;
> > +  }
> > +
> > +   clang::LangStandard::Kind
> > +   get_language_version(const std::vector<std::string> &opts,
> > +                        const std::string &device_version) {
> > +
> > +      const std::string search = "-cl-std=CL";
> > +
> > +       for(auto opt: opts){
> > +           auto pos = opt.find(search);
> > +           if (pos == 0){
> > +               auto ver = opt.substr(pos+search.size());
> > +               return get_language_from_version_str(ver, true);
> > +           }
> > +       }
> > +
> > +       return get_language_from_version_str(device_version);
> > +    }
> > +
> >     std::unique_ptr<clang::CompilerInstance>
> >     create_compiler_instance(const target &target,
> >                              const std::vector<std::string> &opts,
> > @@ -129,7 +171,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_version));
> >  
> >        c->createDiagnostics(new clang::TextDiagnosticPrinter(
> >                                *new raw_string_ostream(r_log),
> > @@ -211,7 +253,7 @@ clover::llvm::compile_program(const std::string &source,
> >  
> >     auto ctx = create_context(r_log);
> >     auto c = create_compiler_instance(target, tokenize(opts + " input.cl"),
> > -                                     r_log);
> > +                                     device_version, r_log);
> 
> This should be part of patch 3 as that patch doesn't build otherwise.
> 
> >     auto mod = compile(*ctx, *c, "input.cl", source, headers, target, opts,
> >                        r_log);
> >  
> > @@ -280,7 +322,7 @@ clover::llvm::link_program(const std::vector<module> &modules,
> >     erase_if(equals("-create-library"), options);
> >  
> >     auto ctx = create_context(r_log);
> > -   auto c = create_compiler_instance(target, options, r_log);
> > +   auto c = create_compiler_instance(target, options, device_version, r_log);
> 
> Same here, this should be in patch 3.
> 
> Thank you,
> Pierre
> 
> >     auto mod = link(*ctx, *c, modules, r_log);
> >  
> >     optimize(*mod, c->getCodeGenOpts().OptimizationLevel, !create_library);
> > -- 
> > 2.11.0
> > 
> > _______________________________________________
> > mesa-dev mailing list
> > mesa-dev at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/mesa-dev
> 
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev

-- 
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/20170722/d63e2a19/attachment.sig>