[Mesa-dev] [PATCH 8/8] clover/llvm: Make __OPENCL_VERSION__ dynamic

[Aaron Watry]

On Thu, Aug 10, 2017 at 11:07 AM, Jan Vesely <jan.vesely at rutgers.edu> wrote:
> On Wed, 2017-08-09 at 22:36 -0500, Aaron Watry wrote:
>> On Fri, Aug 4, 2017 at 1:43 PM, Jan Vesely <jan.vesely at rutgers.edu> wrote:
>> > On Sun, 2017-07-30 at 20:26 -0500, Aaron Watry wrote:
>> > > Signed-off-by: Aaron Watry <awatry at gmail.com>
>> > > CC: Jan Vesely <jan.vesely at rutgers.edu>
>> > >
>> > > v2: base it on the device version
>> > > ---
>> > >  src/gallium/state_trackers/clover/llvm/invocation.cpp | 3 ++-
>> > >  1 file changed, 2 insertions(+), 1 deletion(-)
>> > >
>> > > diff --git a/src/gallium/state_trackers/clover/llvm/invocation.cpp b/src/gallium/state_trackers/clover/llvm/invocation.cpp
>> > > index 63b2961752..443cd31e66 100644
>> > > --- a/src/gallium/state_trackers/clover/llvm/invocation.cpp
>> > > +++ b/src/gallium/state_trackers/clover/llvm/invocation.cpp
>> > > @@ -224,7 +224,8 @@ 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_language_from_version_str(dev.device_version())));
>> >
>> > I don't think you can use the same parsing function here.
>> > __OPENCL_VERSION__ can go up to 2.2, while __OPENCL_C_VERSION__ is max
>> > 2.0
>>
>> Is that an issue here? I thought that the device's highest supported
>> OpenCL version was what was required here?
>
> The problem is that 'get_language_from_version_str' throws exception
> when the version is >2.0. It's OK for clc version, but device CL
> version can go above that.
> I don't think it's a big problem, a simple "TODO: consider higher
> versions" might be enough for now. it will take a while for clover to
> actually run into this.
>
>>
>> __OPENCL_VERSION__ is defined as "substitutes an integer reflecting
>> the version number of the OpenCL
>> supported by the OpenCL device", which in this case should map
>> directly to dev.device_version().
>>
>> __OPENCL_C_VERSION__ is already added by clang when the pre-processor
>> is initialized using the selected language version
>> (clang/FrontEnd/InitPreprocessor.cpp).
>
> The more I think about this (default CLC version, and language version
> restrictions), the more it looks like something that should be handled
> by clang. The information is conceptually of the same kind as the set
> of supported extensions.
> My idea is to add default clc setting to clang/lib/Basic/Targets/, and
> have a "Warning: CLC version incompatible with selected target" if the
> invocation sets something that the target device does not support.
> Clover can add "-Werror=clc-incompatible-version" to enforce build
> failures.
>
> I'm not sure if clang people will like the idea of having target
> dependent default language version.

We also would ideally also want to have some way of supporting the
existing LLVM/clang version as well, and then use whatever clang
provides whenever it's there.

Given that the list of supported extensions is determined by the
run-time in some cases (e.g. are popcount/printf implemented in libclc
when determining to expose 1.2), I'm not sure if the
version/capabilities are entirely something that can/should be
determined by clang.

--Aaron

>
> Jan
>
>>
>> --Aaron
>>
>> >
>> > Jan
>> >
>> > >
>> > >        // clc.h requires that this macro be defined:
>> > >        c.getPreprocessorOpts().addMacroDef("cl_clang_storage_class_specifiers");
>>
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev