[DOC][ABI-BREAK] Remove bfloat16 math aspect.

[JackAKirk]

This aspect is not required because bfloat16 math functions are implemented for all devices via generic implementations. This PR updates this status inline with the main bfloat16 extension/doc.

[gmlueck]

Changes to the doc look good.

Will you remove the aspect from the headers in a separate PR?

[JackAKirk]

Changes to the doc look good.

Will you remove the aspect from the headers in a separate PR?

I grep'd for it and as far as I can tell it has already been completely removed? Also in Unified-Runtime.

[gmlueck]

Oh, I think there was a typo with the name of the aspect in the doc. If you grep for the correct name:

$ ff ext_oneapi_bfloat16_math_functions
./sycl/device_aspect_macros.hpp:#ifndef __SYCL_ALL_DEVICES_HAVE_ext_oneapi_bfloat16_math_functions__
./sycl/device_aspect_macros.hpp:// __SYCL_ASPECT(ext_oneapi_bfloat16_math_functions, 35)
./sycl/device_aspect_macros.hpp:#define __SYCL_ALL_DEVICES_HAVE_ext_oneapi_bfloat16_math_functions__ 0
./sycl/device_aspect_macros.hpp:#ifndef __SYCL_ANY_DEVICE_HAS_ext_oneapi_bfloat16_math_functions__
./sycl/device_aspect_macros.hpp:// __SYCL_ASPECT(ext_oneapi_bfloat16_math_functions, 35)
./sycl/device_aspect_macros.hpp:#define __SYCL_ANY_DEVICE_HAS_ext_oneapi_bfloat16_math_functions__ 0
./sycl/info/aspects.def:__SYCL_ASPECT(ext_oneapi_bfloat16_math_functions, 35)
./sycl/info/device_traits.def:__SYCL_PARAM_TRAITS_SPEC(device, ext_oneapi_bfloat16_math_functions, bool,

[JackAKirk]

Oh, I think there was a typo with the name of the aspect in the doc. If you grep for the correct name:

$ ff ext_oneapi_bfloat16_math_functions
./sycl/device_aspect_macros.hpp:#ifndef __SYCL_ALL_DEVICES_HAVE_ext_oneapi_bfloat16_math_functions__
./sycl/device_aspect_macros.hpp:// __SYCL_ASPECT(ext_oneapi_bfloat16_math_functions, 35)
./sycl/device_aspect_macros.hpp:#define __SYCL_ALL_DEVICES_HAVE_ext_oneapi_bfloat16_math_functions__ 0
./sycl/device_aspect_macros.hpp:#ifndef __SYCL_ANY_DEVICE_HAS_ext_oneapi_bfloat16_math_functions__
./sycl/device_aspect_macros.hpp:// __SYCL_ASPECT(ext_oneapi_bfloat16_math_functions, 35)
./sycl/device_aspect_macros.hpp:#define __SYCL_ANY_DEVICE_HAS_ext_oneapi_bfloat16_math_functions__ 0
./sycl/info/aspects.def:__SYCL_ASPECT(ext_oneapi_bfloat16_math_functions, 35)
./sycl/info/device_traits.def:__SYCL_PARAM_TRAITS_SPEC(device, ext_oneapi_bfloat16_math_functions, bool,

I see thanks. OK I'll go through and remove all these.

[AlexeySachkov]

llvm-spirv changes should be done in the upstream translator PR, even though I don't think that they are necessary. Perhaps an even better change would be to drop that metadata from those tests completely, because it has nothing to do with SPIR-V translation.

llvm changes LGTM

[JackAKirk]

@gmlueck removing this aspect check reveals that CI is failing for intel 11th gen cpu with:

__devicelib_ConvertBF16ToFINTEL [passing parameter 1 with incompatible type]
# | Linking failed
# |  -17 (PI_ERROR_LINK_PROGRAM_FAILURE)

This would seem to be because this backend is choosing the path of some native bf16 specializations irrespective of arch generation. I presume that for AMX cpus this would pass, but I guess this should also be verified.

I see that the aspect check is now made via UR_DEVICE_INFO_BFLOAT16 in unified runtime. L0 is also marked as not supporting this device aspect: https://github.com/oneapi-src/unified-runtime/blob/1473ed8a81ca86e67321dfc90c79e951ead212ac/source/adapters/level_zero/device.cpp#L792 . It looks like we will have to keep this aspect for now.

[JackAKirk]

@gmlueck removing this aspect check reveals that CI is failing for intel 11th gen cpu with:

__devicelib_ConvertBF16ToFINTEL [passing parameter 1 with incompatible type]
# | Linking failed
# |  -17 (PI_ERROR_LINK_PROGRAM_FAILURE)

This would seem to be because this backend is choosing the path of some native bf16 specializations irrespective of arch generation. I presume that for AMX cpus this would pass, but I guess this should also be verified.

I see that the aspect check is now made via UR_DEVICE_INFO_BFLOAT16 in unified runtime. L0 is also marked as not supporting this device aspect: https://github.com/oneapi-src/unified-runtime/blob/1473ed8a81ca86e67321dfc90c79e951ead212ac/source/adapters/level_zero/device.cpp#L792 . It looks like we will have to keep this aspect for now.

Actually the problem is not specific to the math builtins test. It is the same issue that has cpu marked xfail on the bfloat16 type/conversions tests. So it probably does make sense to remove the aspect and then mark the bfloat16 math test xfail on the cpu backend to match the status of the rest of the bfloat16 functions.

[gmlueck]

Actually the problem is not specific to the math builtins test. It is the same issue that has cpu marked xfail on the bfloat16 type/conversions tests. So it probably does make sense to remove the aspect and then mark the bfloat16 math test xfail on the cpu backend to match the status of the rest of the bfloat16 functions.

I thought we had logic to emulate the BF16 operations when they are not supported natively on the device. @rdeodhar might remember more.

[rdeodhar]

Actually the problem is not specific to the math builtins test. It is the same issue that has cpu marked xfail on the bfloat16 type/conversions tests. So it probably does make sense to remove the aspect and then mark the bfloat16 math test xfail on the cpu backend to match the status of the rest of the bfloat16 functions.

I thought we had logic to emulate the BF16 operations when they are not supported natively on the device. @rdeodhar might remember more.

When bfloat16 support was first implemented there were "bfloat16 native" and "bfloat16 emulation" libraries. When JITing, a runtime aspect check was made and depending on bfloat16 availability, one or the other library was linked in. In AOT mode, where a specific device was specified, the knowledge of which devices supported bfloat16 was known to the clang Driver and it linked in the proper library. The implementation has gone through some change and I don't know for sure how it is handled now.

[JackAKirk]

Actually the problem is not specific to the math builtins test. It is the same issue that has cpu marked xfail on the bfloat16 type/conversions tests. So it probably does make sense to remove the aspect and then mark the bfloat16 math test xfail on the cpu backend to match the status of the rest of the bfloat16 functions.

I thought we had logic to emulate the BF16 operations when they are not supported natively on the device. @rdeodhar might remember more.

When bfloat16 support was first implemented there were "bfloat16 native" and "bfloat16 emulation" libraries. When JITing, a runtime aspect check was made and depending on bfloat16 availability, one or the other library was linked in. In AOT mode, where a specific device was specified, the knowledge of which devices supported bfloat16 was known to the clang Driver and it linked in the proper library. The implementation has gone through some change and I don't know for sure how it is handled now.

Yeah it seems that the switch to opaque pointers #9828 meant that the fallback bfloat16 conversions are not working/being called correctly.

So it will make sense to remove the aspect since there is nothing special about the math function failures on some intel cpus because it is due to the same opaque pointer bfloat16 conversion failure.

[JackAKirk]

@KseniyaTikhomirova It would be great to get a review/merge this asap, since the abi break window is now open.

Thanks

[KseniyaTikhomirova]

LGTM

[JackAKirk]

@intel/llvm-gatekeepers please merge.

windows failure is a unrelated CI issue seen in other PRs

[sarnex]

The windows failure preventing the tests from running at all, I'd rather they run, so I'm restarting the CI.

[JackAKirk]

This is up to date with sycl branch. Please do not cancel if there is no longer an issue in sycl branch as per the latest communication.