[Matrix][SYCL] Add support for bf16's wi_element #5397
Conversation
bader
changed the title
|
@AlexeySotkin @dkhaldi I suggest you creating a team on github and adding it as a code owner for this code to https://github.com/intel/llvm/blob/sycl/.github/CODEOWNERS, much like ESIMD extension is added. @yubingex007-a11y I'll approve this PR if Alexey or Dounia approve |
| @@ -451,6 +451,276 @@ class wi_element { | |||
| } | |||
| }; | |||
|
|
|||
| template <size_t NumRows, size_t NumCols, matrix_layout Layout, typename Group> | |||
| class wi_element<uint16_t, NumRows, NumCols, Layout, Group> { | |||
There was a problem hiding this comment.
I think we should specialize this class on bfloat16 type.
There was a problem hiding this comment.
Alexey, we are adding the support of bfloat16 type as the next step in a separate PR.
This PR is to extend the support of the matrix bf16 type represented by unsigned short.
so we are not changing the way we treat bf16 right now.
Instead, we plan on keeping both: unsigned short as bf16, then SYCL bfloat16 type.
We will keep both until the SYCL bfloat16 makes it to the specification (it is experimental right now)
There was a problem hiding this comment.
@AlexeySotkin @dkhaldi For now we still consider uint16_t as bf16 in matrix and in the future we will support sycl's bf16.
| static float make_fp32(uint16_t x) { | ||
| unsigned int y = x; | ||
| y = y << 16; | ||
| float *res = reinterpret_cast<float *>(&y); | ||
| return *res; | ||
| } | ||
|
|
||
| static uint16_t make_bf16(float x) { | ||
| int *res = reinterpret_cast<int *>(&x); | ||
| *res = *res >> 16; | ||
| return (uint16_t)*res; | ||
| } |
There was a problem hiding this comment.
Why we can't use __spirv_ConvertBF16ToFINTEL and __spirv_ConvertFToBF16INTEL here?
There was a problem hiding this comment.
We use this workaround until we support __spirv_ConvertBF16ToFINTEL and __spirv_ConvertFToBF16INTEL in cpu side.
There was a problem hiding this comment.
FYI. I'm working on supporting SYCL_INTEL_bf16_conversion capability on CPU.
| @@ -451,6 +451,276 @@ class wi_element { | |||
| } | |||
| }; | |||
|
|
|||
| template <size_t NumRows, size_t NumCols, matrix_layout Layout, typename Group> | |||
There was a problem hiding this comment.
add a comment that uint16_t here is representing bf16 type as we have been doing for the previous matrix functions.
Since the AMX and DPAS implementations don't support uint16_t, this should raise no problem.
Add a comment that the plan is to move towards SYCL bfloat16 as it makes it to the specification.
| @@ -451,6 +451,284 @@ class wi_element { | |||
| } | |||
| }; | |||
|
|
|||
| // uint16_t here represents bf16 type as we have been doing for the | |||
There was a problem hiding this comment.
Some rewording:
// Note that similarly to the other matrix functions, uint16_t is used here to represent bf16 type. Since the AMX and DPAS implementations don't support uint16_t, this interpretation is possible. This design choice was made before the introduction of SYCL experimental bfloat16 type. Our plan is to move towards using the SYCL bfloat16. But since it is still experimental, we will probably keep both uint16 interpretation and SYCL bfloat16.
| } | ||
|
|
||
| // For now we use the following functions for convertion(bf16=>fp32, | ||
| // fp32=>bf16) as a workaround. In the future we will use |
There was a problem hiding this comment.
Some rewording:
// We use here the following functions for conversion (bf16=>fp32 and fp32=>bf16). This is a workaround until we are able to use __spirv_ConvertFToBF16INTEL and __spirv_ConvertBF16ToFINTEL once these are supported in the CPU backend
|
ping? @intel/llvm-reviewers-runtime |
|
@alexbatashev Hi, could you approve and merge it? |
There was a problem hiding this comment.
@bader or @vladimirlaz or @dm-vodopyanov can you please merge?
Previously, we didn't take bf16's operation into consideration. Now we add support for bf16's wi_element and when we have operations of bf16, we extend to fp32 and do the operations.