PI Plugin Versioning

[AerialMantis]

I've been considering proposing the idea of having versioning for PI plugins, along the lines of the SYCL runtime library versioning (https://github.com/intel/llvm/blob/sycl/sycl/doc/PreprocessorMacros.md#version-macros) so that the version of specific backends can increment and be tracked independent of the SYCL runtime version.

Primarily I was considering this for the CUDA and HIP backends, however, I thought it would make sense if there was a consistent mechanism for this across all backends in DPC++.

What are other people's thoughts on this?

@intel/llvm-reviewers-runtime

[pvchupin]

Tagging @smaslov-intel

[smaslov-intel]

Yes, that was the original intention, see

llvm/sycl/include/CL/sycl/detail/pi.h

Line 1765 in 844d7b6

char PluginVersion[4];

We should also connect plugin's version with it's supported PI API version, such that incompatibilities are detected and clearly reported.
The PI API versioning is here:

llvm/sycl/include/CL/sycl/detail/pi.h

Line 25 in 844d7b6

// Every single change in PI API should be accompanied with the minor

[AerialMantis]

That's interesting thanks @smaslov-intel, so as I understand PiVersion is the current version of the PI API and PluginVersion is the version of the plugin, but it looks as though these aren't used in the plugins atm, is that right?

Also would this mechanism be able to track changes to PI plugins not necessarily related to conforming to the PI API, for bug fixes, etc, if not perhaps we could have a patch component as well, which doesn't correspond to the PI API version?

[smaslov-intel]

There is some trivial use of PiVersion already that prevents using plugin's with a different version of PI API, e.g.:

llvm/sycl/plugins/opencl/pi_opencl.cpp

Line 1475 in fc0d28a

int CompareVersions = strcmp(PluginInit->PiVersion, SupportedVersion);

It can be improved by recognizing minor/major version change and only reject plugins that have backward incompatible differences.

There is also some use of PluginVersion, e.g. here:

llvm/sycl/plugins/esimd_emulator/pi_esimd_emulator.cpp

Line 1748 in fc0d28a

strncpy(PluginInit->PluginVersion, _PI_H_VERSION_STRING, PluginVersionSize);

But it is also sporadic and not very well defined. I suggest that we define unified versioning across plugins and start using that:

llvm/sycl/include/CL/sycl/detail/pi.h

Line 1765 in 844d7b6

char PluginVersion[4];

Today, the PiVersion is a string "major.minor" ("6.8" currently):

llvm/sycl/include/CL/sycl/detail/pi.h

Line 52 in fc0d28a

#define _PI_H_VERSION_STRING \

Maybe we can define PluginVersion to be of the form "major.minor.ordinal", where "major.minoir" repeats PI API version, and "ordinal" is a plugin-specific ordinal number of the last change-set in that plugin (with the only requirement is that it should be monotonically increasing)?

[AerialMantis]

Thanks for your reply and explanation @smaslov-intel, apologies I'm late in getting back to you on this. We've looked into this and we have a good idea of what we need to do to enable the PI plugin version for PI CUDA.

We've also been thinking some more about the ordinal component to version, I think this is a good idea, originally my thinking was that this could be used for users to identify changes to the backend between SYCL runtime versions, however, as the PI API is an implementation detail we concluded it's not particularly meaningful to users. Though I think it would still be useful though to have this component for internal use to indicate patches to a plugin.

We plan to implement this in PI CUDA, though it's not on our immediate roadmap atm.