[Mesa-dev] feature levels in clover (was: Re: [PATCH 2/2] clover: Query and export int64 atomics)

Fri Sep 29 15:56:04 UTC 2017

On Wed, 2017-09-27 at 11:25 -0700, Francisco Jerez wrote:
> Jan Vesely <jan.vesely at rutgers.edu> writes:
> 
> > On Tue, 2017-09-26 at 14:51 -0700, Francisco Jerez wrote:
> > > Jan Vesely <jan.vesely at rutgers.edu> writes:
> > > 
> > > > On Wed, 2017-09-20 at 19:10 -0500, Aaron Watry wrote:
> > > > > [SNIP]
> > > > > 
> > > > > Not trying to rain on your parade, but I've been thinking that we
> > > > > might need to be able to query the installed libclc version or
> > > > > possibly consider querying the libclc library for information about
> > > > > which extensions are implemented (possibly even for the specific
> > > > > device/target and the llvm version it was built against).
> > > > > 
> > > > > I've been slowly working on a printf implementation, and I'll need to
> > > > > expose that based on some dynamic state of the system as well.
> > > > > 
> > > > > For something that's adding new extension support in libclc, we might
> > > > > at least want to bump the libclc version number and ask Tom what he
> > > > > did in the past related to branching off a libclc release, and then
> > > > > make sure we've got a way to link against libclc (if we're not
> > > > > already) and get its version number.
> > > > 
> > > > I disagree.
> > > > Extensions need to be exposed by both clover (for host code) and clang
> > > > (for clc code), synchronizing those is tricky enough. adding two more
> > > > variables (libclc version, libclc clang build version) will make things
> > > > intractable.
> > > > 
> > > > It's a lot easier (and I'd say common) for applications to check for
> > > > extensions in CLC code (ifdef/ifndef) and handle all failures in
> > > > generic 'build failed' error path. That makes clang the authoritative
> > > > source of supported extensions for each target.
> > > > Ideally, clover would get the list of supported extensions from clang
> > > > (based on device target) and expose those.
> > > > clang has exposed almost all extensions since at least 3.9.
> > > > Implementing and exposing them (including int64 atomics) is thus a
> > > > bugfix, not a feature update.
> > > > 
> > > > I don't want to have libclc release until the library is complete (we
> > > > still miss ~12 builtins + variants). Distros now ship development
> > > > snapshot and providing a released version would make more
> > > > backward^Wconservative distros (ehm, debian) stuck on that version.
> > > > This way we have only one version; HEAD.
> > > > 
> > > > I have posted patches that bring back llvm-3.9 support to libclc, thus
> > > > everyone can use the latest libclcHEAD.
> > > > 
> > > > I agree that we need to differentiate libclc variants that follow
> > > > different version of standard. Not just because of missing features,
> > > > but because not all builtins should be exposed in all versions.
> > > > for example: atomic_* should not be available in clc1.0.
> > > > or the program can do something like this:
> > > > #ifndef extension
> > > > ...custom implementation of extensions function
> > > > #else
> > > > #pragma extension: enable
> > > > #endif
> > > > 
> > > > I'd thus propose to add lang version suffix to libclc.
> > > > *.bc.cl100 -- follows OCL-1.0 (builtins added later are not available)
> > > > *.bc.cl110 -- follows OCL-1.1 (deprecated OCL-1.0 builtins/macros are
> > > > not available, buitlins from 1.2 are not available)
> > > > ...
> > > > Current libclc would thus provide only .cl100 and .cl110 with missing
> > > > builtins/bugs.
> > > > 
> > > > 
> > > > It'd be possible for clover to check presence of such libraries and
> > > > restrict CLC (and CL) feature level accordingly.
> > > > 
> > > 
> > > Wouldn't it be easier for Clover to check which libclc version it's
> > > linking against and expose a subset of language versions and extensions
> > > accordingly?  That should only take a tiny bit of build system support
> > > and wouldn't lock us to a single libclc release per API version (which
> > > would make things tricky if extensions get back-ported by Khronos to
> > > older API versions after the fact, or if people have reasons to delay
> > > the implementation of a subset of extensions after a release).
> > 
> > I don't think libclc version makes sense.
> > 
> > atm, there are still core clc 1.0 builtins missing so we really should
> > not expose even cl1.0
> > 
> > when it comes to extensions, libclc has to implement all builtins for
> > extensions exposed by clang, whether clover exposes the same extension
> > or not, otherwise legal programs fail to build.
> 
> I'd argue instead that we shouldn't be letting Clang expose an extension
> that is missing support code from libclc.

Ideally yes, we'd a have a set of extensions supported by clang,
clover, and libclc. Then we could expose intersection of all 3 sets in
both clover and clang.

We'd need to pass information both to and from clover. I've been
arguing for the latter. The former seems like an extra effort to hide
functionality that you'll implement anyway. Moreover, it postpones
availability of new functinality until both new libclc version is out
and a new mesa versions (that recognizes the released libclc) is out.

> 
> > I would not expect an application to check clGetDeviceInfo for
> > extensions and then pass custom defines to kernel build (that's what
> > ifdef cl_khr_* is for).
> 
> Why not?  It certainly seems useful to me for the host to have an
> accurate view of what extensions are supported by the language.
> Sometimes working around for a missing extension implies different host
> behavior (e.g. if 64bit atomics are not supported there may be a 32bit
> fall-back path that requires splitting up work into smaller pieces to
> avoid 32bit overflow), or providing completely different kernels.

Agreed, host side checking has its uses. It is, however, subset of CLC
side checks, since kernels that cannot be invoked cannot be built
either. In addition to that you can have code like (from docs example):
#inef cl_khr_EXTENSION
// Implement efficient functions that use the extension
#else
// Implement workaround for missing extension
#endif
which does not change host side execution.

granted, you can pass different input source based host extension
checks, but I wouldn't expect applications to do that since ifdef
checks provide a more convenient way to do that.

> 
> > Moreover, libclc is the easiest one to upgrade because nothing else in
> > the system depends on it (as opposed to llvm and mesa).
> > 
> > The idea behind liblc per language variants is that the user should be
> > able to use function names that were later taken for builtins (like
> > shuffle).
> 
> I don't think that tracking libclc versions would be in conflict with
> that.

it has a potential to make the entire setup more complicated if you
need to track version across 4 different language variants. Hopefully
we won't need that.

Jan

> 
> > we might be able to get away with a single library variant if users
> > don't use overloadable attribute.
> > 
> > > 
> > > Either way, patch looks good to me:
> > > 
> > > Reviewed-by: Francisco Jerez <currojerez at riseup.net>
> > 
> > thanks, pushed.
> > Jan
> > 
> > > 
> > > > Jan
> > > > 
> > > > [SNIP]
> > > > -- 
> > > > Jan Vesely <jan.vesely at rutgers.edu>
> > > > _______________________________________________
> > > > mesa-dev mailing list
> > > > mesa-dev at lists.freedesktop.org
> > > > https://lists.freedesktop.org/mailman/listinfo/mesa-dev
-------------- 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/20170929/5b097736/attachment.sig>