[SYCL][CUDA][HIP] Fix PI version reporting

[npmiller]

Report the actual PI version rather than 0.0.

[steffenlarsen]

This added information is definitely more informative than previously, but I wonder if it would be more in line with PI_DEVICE_INFO_VERSION to report the compute capabilities of the CUDA device instead, i.e. cuDeviceGetAttribute with CU_DEVICE_ATTRIBUTE_COMPUTE_CAPABILITY_MAJOR and CU_DEVICE_ATTRIBUTE_COMPUTE_CAPABILITY_MINOR. See CL_DEVICE_VERSION in clGetDeviceInfo.

I don't know if HIP has a similar info query.

[alexbatashev]

@npmiller there was a fail for OCL CPU job, which was caused by machine misconfiugration. I fixed that and restarted the job for you. Should be fine now.

[npmiller]

What the device version is meant to be is pretty confusing to me, it seems that it could be backend specific.

For info::device::version the SYCL spec says:

Returns the SYCL version as a std::string in the form: <major_version>.<minor_version>.

• https://www.khronos.org/registry/SYCL/specs/sycl-2020/html/sycl-2020.html#_device_information_descriptors

But it does mention in that section that the meaning of any property can be redefined in the backend spec, and in the OpenCL backend spec it says the following:

The device version is an indication of the device’s capabilities, as represented by the device information returned by the sycl::device::get_info() member function. Examples of attributes associated with the device version are resource limits and information about functionality beyond the requirements in the core SYCL specification. The version returned corresponds to the highest version of the OpenCL specification for which the device is conformant, but is not higher than the version of the device’s platform which bounds the overall capabilities of the runtime operating the device.

• https://www.khronos.org/registry/SYCL/specs/sycl-2020/html/sycl-2020.html#_platform_mixed_version_support_2

Which also sounds a bit contradicting but suggests that it should be the highest OpenCL version reported there.

And in fact AMD OpenCL devices report 2.0 for this field, which is likely to be OpenCL 2.0.

In OpenCL the same version property is defined as OpenCL<space><major_version.minor_version><space><vendor-specific information>

For level zero it's set to the LevelZero API version which is seemingly different from the LevelZero driver version.

• https://github.com/intel/llvm/blob/sycl/sycl/plugins/level_zero/pi_level_zero.cpp#L2396

So all of that to say I'm really not sure what's the best thing to report here, I think the PI version makes quite a bit of sense when compared to the OpenCL backend, but it doesn't seem very helpful, compute capabilities would make a lot of sense but it doesn't seem like any other backend uses the device version that way.

What do you think?

As a side note, HIP does have similar compute capabilities version numbers but discourages using them, and advises to use the feature flags instead (AMD HIP API 4.5, section 5.39.2.6):

Major compute capability. On HCC, this is an approximation and features may differ from CUDA CC. See the arch
feature flags for portable ways to query feature caps.

[steffenlarsen]

@npmiller - Thank you for the thorough analysis! I agree, the wording and intent of the info query is a little vague.

I see what you mean, that it is hard to draw a comparison between the compute capability and the API versions reported by the other plugins. I am not opposed to reporting the PI version here, but my thinking is that reporting the compute capability gives the user more device-specific information.
Maybe it could report both in the same query? E.g. "PI X.Y (Compute capability A.B)". Since users need to explicitly set SM version higher than sm_50 (?) it could be useful information which I believe the user can get through simply running sycl-ls (assuming this information is added to the query).

[smaslov-intel]

I am not opposed to reporting the PI version here, but my thinking is that reporting the compute capability gives the user more device-specific information.

Additionally PI API version is available other ways, and reporting native device's API version is not.

[npmiller]

I see, yeah that makes sense it would be really helpful. I think it should be fine to just put the useful data in this field, I've updated it so that it gives the following output:

CUDA plugin and HIP Nvidia plugin:

Platform [#3]:
    Version  : CUDA 11.6
    Name     : NVIDIA CUDA BACKEND
    Vendor   : NVIDIA Corporation
    Devices  : 1
        Device [#0]:
        Type       : gpu
        Version    : Compute Capability 6.6
        Name       : NVIDIA GeForce GTX 1050 Ti
        Vendor     : NVIDIA Corporation
        Driver     : CUDA 11.6
Platform [#4]:
    Version  : HIP 0.0
    Name     : AMD HIP BACKEND
    Vendor   : AMD Corporation
    Devices  : 1
        Device [#0]:
        Type       : gpu
        Version    : Compute Capability 6.1
        Name       : NVIDIA GeForce GTX 1050 Ti
        Vendor     : AMD Corporation
        Driver     : HIP 0.0

HIP AMD plugin:

Platform [#2]:          
    Version  : HIP 40421.43          
    Name     : AMD HIP BACKEND
    Vendor   : AMD Corporation
    Devices  : 1                                                                                                                                                             
        Device [#0]:                                                                  
        Type       : gpu                                                              
        Version    : gfx908:sramecc+:xnack-                                    
        Name       :                                                                                                                                                         
        Vendor     : AMD Corporation                                                                                                                                         
        Driver     : HIP 40421.43  

It's going to lead to a bit of duplication on AMD with #5508 but I don't think that's a big deal.

I had to update the function that was returning the device version string as all the plugins were going through the OpenCL path which trimmed the string we reported in the plugins down to just the numerical version.

[npmiller]

So after further discussions and reviewing the current SYCL specification, I've decided to file a ticket with the SYCL specification to seek clarifications on this:

• Clarify what the device version property should report KhronosGroup/SYCL-Docs#222

So I'll put this PR on hold until we get some clarifications on what this property is meant to be and what we're allowed to use it for.

[npmiller]

The SYCL specification PR has now been merged so now the device version is backend defined, so we should be fine to report the compute capabilities and define it as such in the CUDA backend specification. The CUDA backend specification is not finalized yet so this may need to be tweaked later on but it should be fine for now.

[steffenlarsen]

LGTM!

[npmiller]

Updated to remove the extra processing on OpenCL version strings to extract just the version number, the full version can now be returned directly, deferring the formatting to the OpenCL spec.

This is a change in behavior but the previous behavior was incorrect as it should have been the SYCL version returned not the OpenCL version. And is reflected in the spec changes of KhronosGroup/SYCL-Docs#231

[npmiller]

/verify with intel/llvm-test-suite#1019

[npmiller]

All tests failures are expected, and are fixed by:

• [SYCL] Fix tests using device version llvm-test-suite#1019

And for one of the AMD failure by:

• [SYCL] Check that fp16 aspect is supported before using half llvm-test-suite#1487

[npmiller]

/verify with intel/llvm-test-suite#1019

[npmiller]

ping @steffenlarsen @smaslov-intel

Sorry I keep forgetting about this PR, are we good to merge this and the matching tests PR: intel/llvm-test-suite#1019 ?

Or does it need more reviews and/or rebasing?

[steffenlarsen]

LGTM! @npmiller - Could you please push a merge commit to make sure it works with tip.

[npmiller]

/verify with intel/llvm-test-suite#1019

[steffenlarsen]

/verify with intel/llvm-test-suite#1019

[steffenlarsen]

@npmiller - It looks like there are new failing tests. Could you please address these?

[npmiller]

/verify with intel/llvm-test-suite#1019

[npmiller]

/verify with intel/llvm-test-suite#1019

[npmiller]

/verify with intel/llvm-test-suite#1019

[npmiller]

/verify with intel/llvm-test-suite#1019

[npmiller]

/verify with intel/llvm-test-suite#1019

[bader]

@npmiller, please, update your branch and move tests from intel/llvm-test-suite#1019 to sycl/test-e2e.
This way they will be tested on HIP backend before the merge!