New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[AMDGPU] Incorrect parsing of bf16 literals #79369

Closed

rampitec opened this issue Jan 24, 2024 · 1 comment · Fixed by #80908

Assignees

Labels

Collaborator

rampitec commented Jan 24, 2024 •

edited

Loading

bf16 immediate operands are not handled correctly by asm parser (but seem OK in the codegen):

llvm-mc -arch=amdgcn -mcpu=gfx1200 -show-encoding <<< 'v_dot2_bf16_bf16 v5, v1, v2, 100.0'
v_dot2_bf16_bf16 v5, v1, v2, 0x5640     ; encoding: [0x05,0x00,0x67,0xd6,0x01,0x05,0xfe,0x03,0x40,0x56,0x00,0x00]

bf16 constants are essentially fp32 with all zero low 16 bits. So 100.0 shall be encoded as 0x42c80000, and since we only accept 16 bits in the asm hex for it has to be 0x42c8.

llvm-mc -arch=amdgcn -mcpu=gfx1200 -show-encoding <<< 'v_dot2_bf16_bf16 v5, v1, v2, 1.0'
v_dot2_bf16_bf16 v5, v1, v2, 0x3c00     ; encoding: [0x05,0x00,0x67,0xd6,0x01,0x05,0xfe,0x03,0x00,0x3c,0x00,0x00]

This shall be inline immediate.

Basically we are parsing bf16 constants as f16.

The text was updated successfully, but these errors were encountered:

github-actions bot added the new issue label

rampitec added the backend:AMDGPU label

Collaborator

llvmbot commented Jan 24, 2024

@llvm/issue-subscribers-backend-amdgpu

Author: Stanislav Mekhanoshin (rampitec)

bf16 immediate operands are not handled correctly by asm parser (but seem OK in the codegen): ``` llvm-mc -arch=amdgcn -mcpu=gfx1200 -show-encoding <<< 'v_dot2_bf16_bf16 v5, v1, v2, 100.0' v_dot2_bf16_bf16 v5, v1, v2, 0x5640 ; encoding: [0x05,0x00,0x67,0xd6,0x01,0x05,0xfe,0x03,0x40,0x56,0x00,0x00] ``` bf16 constants are essentially fp32 with all zero low 16 bits. So 100.0 shall be encoded as 0x42c80000, and since we only accept 16 bits in the asm hex for it has to be 0x42c8. ``` llvm-mc -arch=amdgcn -mcpu=gfx1200 -show-encoding <<< 'v_dot2_bf16_bf16 v5, v1, v2, 1.0' v_dot2_bf16_bf16 v5, v1, v2, 0x3c00 ; encoding: [0x05,0x00,0x67,0xd6,0x01,0x05,0xfe,0x03,0x00,0x3c,0x00,0x00] ``` This shall be inline immediate.

Basically we a parsing bf16 constants as f16.

EugeneZelenko removed the new issue label

kzhuravl assigned shiltian

shiltian mentioned this issue

[AMDGPU] Use bf16 instead of i16 for bfloat #80908

Merged

shiltian added a commit to shiltian/llvm-project that referenced this issue


          [RFC][WIP][AMDGPU] Use bf16 instead of i16 for bfloat

f8de342

Currently it looks like we generally use `i16` to represent `bf16` in those tablegen
files. I'm not sure of the reason behind it. My wild guess is the type `bf16` was
not available when we enabled the support. This patch is trying to use `bf16`
directly in those tablegen files, aiming at fixing llvm#79369. Of course for llvm#79369
a workaround can be to treat all `INT16` variants as `BFloat` in `getOpFltSemantics`,
but it doesn't look good IMHO.

Since I'm fairly new to AMDGPU backend, I'd appreciate it if you can point out
where I don't understand correctly.

shiltian added a commit to shiltian/llvm-project that referenced this issue


          [RFC][WIP][AMDGPU] Use bf16 instead of i16 for bfloat

7b18520

Currently it looks like we generally use `i16` to represent `bf16` in those tablegen
files. I'm not sure of the reason behind it. My wild guess is the type `bf16` was
not available when we enabled the support. This patch is trying to use `bf16`
directly in those tablegen files, aiming at fixing llvm#79369. Of course for llvm#79369
a workaround can be to treat all `INT16` variants as `BFloat` in `getOpFltSemantics`,
but it doesn't look good IMHO.

Since I'm fairly new to AMDGPU backend, I'd appreciate it if you can point out
where I don't understand correctly.

shiltian added a commit to shiltian/llvm-project that referenced this issue


          [RFC][WIP][AMDGPU] Use bf16 instead of i16 for bfloat

a535bf3

Currently it looks like we generally use `i16` to represent `bf16` in those tablegen
files. I'm not sure of the reason behind it. My wild guess is the type `bf16` was
not available when we enabled the support. This patch is trying to use `bf16`
directly in those tablegen files, aiming at fixing llvm#79369. Of course for llvm#79369
a workaround can be to treat all `INT16` variants as `BFloat` in `getOpFltSemantics`,
but it doesn't look good IMHO.

Since I'm fairly new to AMDGPU backend, I'd appreciate it if you can point out
where I don't understand correctly.

shiltian added a commit to shiltian/llvm-project that referenced this issue


          [RFC][WIP][AMDGPU] Use bf16 instead of i16 for bfloat

6a2bace

Currently it looks like we generally use `i16` to represent `bf16` in those tablegen
files. I'm not sure of the reason behind it. My wild guess is the type `bf16` was
not available when we enabled the support. This patch is trying to use `bf16`
directly in those tablegen files, aiming at fixing llvm#79369. Of course for llvm#79369
a workaround can be to treat all `INT16` variants as `BFloat` in `getOpFltSemantics`,
but it doesn't look good IMHO.

Since I'm fairly new to AMDGPU backend, I'd appreciate it if you can point out
where I don't understand correctly.

shiltian added a commit to shiltian/llvm-project that referenced this issue


          [RFC][WIP][AMDGPU] Use bf16 instead of i16 for bfloat

672fd3c

Currently it looks like we generally use `i16` to represent `bf16` in those tablegen
files. I'm not sure of the reason behind it. My wild guess is the type `bf16` was
not available when we enabled the support. This patch is trying to use `bf16`
directly in those tablegen files, aiming at fixing llvm#79369. Of course for llvm#79369
a workaround can be to treat all `INT16` variants as `BFloat` in `getOpFltSemantics`,
but it doesn't look good IMHO.

Since I'm fairly new to AMDGPU backend, I'd appreciate it if you can point out
where I don't understand correctly.

shiltian added a commit to shiltian/llvm-project that referenced this issue


          [RFC][WIP][AMDGPU] Use bf16 instead of i16 for bfloat

d14668f

Currently it looks like we generally use `i16` to represent `bf16` in those tablegen
files. I'm not sure of the reason behind it. My wild guess is the type `bf16` was
not available when we enabled the support. This patch is trying to use `bf16`
directly in those tablegen files, aiming at fixing llvm#79369. Of course for llvm#79369
a workaround can be to treat all `INT16` variants as `BFloat` in `getOpFltSemantics`,
but it doesn't look good IMHO.

Since I'm fairly new to AMDGPU backend, I'd appreciate it if you can point out
where I don't understand correctly.

shiltian added a commit to shiltian/llvm-project that referenced this issue


          [AMDGPU] Remove unused functions for checking 16-bit inline literals

cc19406

This patch removes unused functions that check if an immediate is a 16-bit inline
literals. This serves as prime patches to fix llvm#79369.

shiltian mentioned this issue

[AMDGPU] Clean up functions for checking inline literals #81282

Merged

shiltian added a commit to shiltian/llvm-project that referenced this issue


          [AMDGPU] Remove unused functions for checking 16-bit inline literals

0d45bcf

This patch removes unused functions that check if an immediate is a 16-bit inline
literals. This serves as prime patches to fix llvm#79369.

shiltian added a commit to shiltian/llvm-project that referenced this issue


          [AMDGPU] Remove unused functions for checking 16-bit inline literals

0f4a871

This patch removes unused functions that check if an immediate is a 16-bit inline
literals. This serves as prime patches to fix llvm#79369.

shiltian added a commit to shiltian/llvm-project that referenced this issue


          [AMDGPU] Remove unused functions for checking 16-bit inline literals

df30665

This patch removes unused functions that check if an immediate is a 16-bit inline
literals. This serves as prime patches to fix llvm#79369.

shiltian added a commit to shiltian/llvm-project that referenced this issue


          [AMDGPU] Remove unused functions for checking 16-bit inline literals

f14cb53

This patch removes unused functions that check if an immediate is a 16-bit inline
literals. This serves as prime patches to fix llvm#79369.

shiltian added a commit to shiltian/llvm-project that referenced this issue


          [RFC][WIP][AMDGPU] Use bf16 instead of i16 for bfloat

4196e99

Currently it looks like we generally use `i16` to represent `bf16` in those tablegen
files. I'm not sure of the reason behind it. My wild guess is the type `bf16` was
not available when we enabled the support. This patch is trying to use `bf16`
directly in those tablegen files, aiming at fixing llvm#79369. Of course for llvm#79369
a workaround can be to treat all `INT16` variants as `BFloat` in `getOpFltSemantics`,
but it doesn't look good IMHO.

Since I'm fairly new to AMDGPU backend, I'd appreciate it if you can point out
where I don't understand correctly.

shiltian added a commit to shiltian/llvm-project that referenced this issue


          [RFC][WIP][AMDGPU] Use bf16 instead of i16 for bfloat

df3dbb6

Currently it looks like we generally use `i16` to represent `bf16` in those tablegen
files. I'm not sure of the reason behind it. My wild guess is the type `bf16` was
not available when we enabled the support. This patch is trying to use `bf16`
directly in those tablegen files, aiming at fixing llvm#79369. Of course for llvm#79369
a workaround can be to treat all `INT16` variants as `BFloat` in `getOpFltSemantics`,
but it doesn't look good IMHO.

Since I'm fairly new to AMDGPU backend, I'd appreciate it if you can point out
where I don't understand correctly.

shiltian added a commit to shiltian/llvm-project that referenced this issue


          [AMDGPU] Remove unused functions for checking 16-bit inline literals

49cf561

This patch removes unused functions that check if an immediate is a 16-bit inline
literals. This serves as prime patches to fix llvm#79369.

shiltian added a commit to shiltian/llvm-project that referenced this issue


          [RFC][WIP][AMDGPU] Use bf16 instead of i16 for bfloat

c556e40

Currently it looks like we generally use `i16` to represent `bf16` in those tablegen
files. I'm not sure of the reason behind it. My wild guess is the type `bf16` was
not available when we enabled the support. This patch is trying to use `bf16`
directly in those tablegen files, aiming at fixing llvm#79369. Of course for llvm#79369
a workaround can be to treat all `INT16` variants as `BFloat` in `getOpFltSemantics`,
but it doesn't look good IMHO.

Since I'm fairly new to AMDGPU backend, I'd appreciate it if you can point out
where I don't understand correctly.

shiltian added a commit to shiltian/llvm-project that referenced this issue


          [AMDGPU] Remove unused functions for checking 16-bit inline literals

e9a5322

This patch removes unused functions that check if an immediate is a 16-bit inline
literals. This serves as prime patches to fix llvm#79369.

shiltian added a commit to shiltian/llvm-project that referenced this issue


          [RFC][WIP][AMDGPU] Use bf16 instead of i16 for bfloat

bfd3170

Currently it looks like we generally use `i16` to represent `bf16` in those tablegen
files. I'm not sure of the reason behind it. My wild guess is the type `bf16` was
not available when we enabled the support. This patch is trying to use `bf16`
directly in those tablegen files, aiming at fixing llvm#79369. Of course for llvm#79369
a workaround can be to treat all `INT16` variants as `BFloat` in `getOpFltSemantics`,
but it doesn't look good IMHO.

Since I'm fairly new to AMDGPU backend, I'd appreciate it if you can point out
where I don't understand correctly.

shiltian added a commit to shiltian/llvm-project that referenced this issue


          [RFC][WIP][AMDGPU] Use bf16 instead of i16 for bfloat

d72bf8b

Currently it looks like we generally use `i16` to represent `bf16` in those tablegen
files. I'm not sure of the reason behind it. My wild guess is the type `bf16` was
not available when we enabled the support. This patch is trying to use `bf16`
directly in those tablegen files, aiming at fixing llvm#79369. Of course for llvm#79369
a workaround can be to treat all `INT16` variants as `BFloat` in `getOpFltSemantics`,
but it doesn't look good IMHO.

Since I'm fairly new to AMDGPU backend, I'd appreciate it if you can point out
where I don't understand correctly.

shiltian added a commit to shiltian/llvm-project that referenced this issue


          [RFC][WIP][AMDGPU] Use bf16 instead of i16 for bfloat

47b96d2

Currently it looks like we generally use `i16` to represent `bf16` in those tablegen
files. I'm not sure of the reason behind it. My wild guess is the type `bf16` was
not available when we enabled the support. This patch is trying to use `bf16`
directly in those tablegen files, aiming at fixing llvm#79369. Of course for llvm#79369
a workaround can be to treat all `INT16` variants as `BFloat` in `getOpFltSemantics`,
but it doesn't look good IMHO.

Since I'm fairly new to AMDGPU backend, I'd appreciate it if you can point out
where I don't understand correctly.

shiltian added a commit to shiltian/llvm-project that referenced this issue


          [RFC][WIP][AMDGPU] Use bf16 instead of i16 for bfloat

7a517ee

Currently it looks like we generally use `i16` to represent `bf16` in those tablegen
files. I'm not sure of the reason behind it. My wild guess is the type `bf16` was
not available when we enabled the support. This patch is trying to use `bf16`
directly in those tablegen files, aiming at fixing llvm#79369. Of course for llvm#79369
a workaround can be to treat all `INT16` variants as `BFloat` in `getOpFltSemantics`,
but it doesn't look good IMHO.

Since I'm fairly new to AMDGPU backend, I'd appreciate it if you can point out
where I don't understand correctly.

shiltian added a commit to shiltian/llvm-project that referenced this issue


          [RFC][WIP][AMDGPU] Use bf16 instead of i16 for bfloat

1488b4e

Currently it looks like we generally use `i16` to represent `bf16` in those tablegen
files. I'm not sure of the reason behind it. My wild guess is the type `bf16` was
not available when we enabled the support. This patch is trying to use `bf16`
directly in those tablegen files, aiming at fixing llvm#79369. Of course for llvm#79369
a workaround can be to treat all `INT16` variants as `BFloat` in `getOpFltSemantics`,
but it doesn't look good IMHO.

Since I'm fairly new to AMDGPU backend, I'd appreciate it if you can point out
where I don't understand correctly.

shiltian added a commit to shiltian/llvm-project that referenced this issue


          [AMDGPU] Remove unused functions for checking 16-bit inline literals

This patch removes unused functions that check if an immediate is a 16-bit inline
literals. This serves as prime patches to fix llvm#79369.

shiltian added a commit to shiltian/llvm-project that referenced this issue


          [RFC][WIP][AMDGPU] Use bf16 instead of i16 for bfloat

784670d

Currently it looks like we generally use `i16` to represent `bf16` in those tablegen
files. I'm not sure of the reason behind it. My wild guess is the type `bf16` was
not available when we enabled the support. This patch is trying to use `bf16`
directly in those tablegen files, aiming at fixing llvm#79369. Of course for llvm#79369
a workaround can be to treat all `INT16` variants as `BFloat` in `getOpFltSemantics`,
but it doesn't look good IMHO.

Since I'm fairly new to AMDGPU backend, I'd appreciate it if you can point out
where I don't understand correctly.

shiltian added a commit to shiltian/llvm-project that referenced this issue


          [AMDGPU] Remove unused functions for checking 16-bit inline literals

This patch removes unused functions that check if an immediate is a 16-bit inline
literals. This serves as prime patches to fix llvm#79369.

shiltian added a commit to shiltian/llvm-project that referenced this issue


          [RFC][WIP][AMDGPU] Use bf16 instead of i16 for bfloat

9fbb1e6

Currently it looks like we generally use `i16` to represent `bf16` in those tablegen
files. I'm not sure of the reason behind it. My wild guess is the type `bf16` was
not available when we enabled the support. This patch is trying to use `bf16`
directly in those tablegen files, aiming at fixing llvm#79369. Of course for llvm#79369
a workaround can be to treat all `INT16` variants as `BFloat` in `getOpFltSemantics`,
but it doesn't look good IMHO.

Since I'm fairly new to AMDGPU backend, I'd appreciate it if you can point out
where I don't understand correctly.

shiltian added a commit to shiltian/llvm-project that referenced this issue


          [RFC][WIP][AMDGPU] Use bf16 instead of i16 for bfloat

d95e99e

Currently it looks like we generally use `i16` to represent `bf16` in those tablegen
files. I'm not sure of the reason behind it. My wild guess is the type `bf16` was
not available when we enabled the support. This patch is trying to use `bf16`
directly in those tablegen files, aiming at fixing llvm#79369. Of course for llvm#79369
a workaround can be to treat all `INT16` variants as `BFloat` in `getOpFltSemantics`,
but it doesn't look good IMHO.

Since I'm fairly new to AMDGPU backend, I'd appreciate it if you can point out
where I don't understand correctly.

shiltian added a commit to shiltian/llvm-project that referenced this issue


          [RFC][WIP][AMDGPU] Use bf16 instead of i16 for bfloat

5b66bb2

Currently it looks like we generally use `i16` to represent `bf16` in those tablegen
files. I'm not sure of the reason behind it. My wild guess is the type `bf16` was
not available when we enabled the support. This patch is trying to use `bf16`
directly in those tablegen files, aiming at fixing llvm#79369. Of course for llvm#79369
a workaround can be to treat all `INT16` variants as `BFloat` in `getOpFltSemantics`,
but it doesn't look good IMHO.

Since I'm fairly new to AMDGPU backend, I'd appreciate it if you can point out
where I don't understand correctly.

shiltian closed this as completed in #80908

shiltian added a commit that referenced this issue


          [AMDGPU] Use bf16 instead of i16 for bfloat (#80908)

46734aa

Currently we generally use `i16` to represent `bf16` in those tablegen
files. This patch is trying to use `bf16` directly.

Fix #79369.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment