Skip to content

Promotion of bfloat is broken for strictfp functions #78540

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.

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

Open
arsenm opened this issue Jan 18, 2024 · 6 comments
Open

Promotion of bfloat is broken for strictfp functions #78540

arsenm opened this issue Jan 18, 2024 · 6 comments
Labels
backend:AMDGPU llvm:SelectionDAG SelectionDAGISel as well

Comments

@arsenm
Copy link
Contributor

arsenm commented Jan 18, 2024

b473864

This disabled a test in llvm/test/CodeGen/AMDGPU/llvm.is.fpclass.bf16 because promotion of bf16 fp_round is broken in strictfp functions. We are currently missing strict variants of BF16_TO_FP and FP_TO_BF16

@arsenm arsenm added backend:AMDGPU llvm:SelectionDAG SelectionDAGISel as well labels Jan 18, 2024
@llvmbot
Copy link
Member

llvmbot commented Jan 18, 2024

@llvm/issue-subscribers-backend-amdgpu

Author: Matt Arsenault (arsenm)

https://github.com//pull/74405/commits/b473864900c68c84e37d68f0ea771ef42f13138a

This disabled a test in llvm/test/CodeGen/AMDGPU/llvm.is.fpclass.bf16 because promotion of bf16 fp_round is broken in strictfp functions. We are currently missing strict variants of BF16_TO_FP and FP_TO_BF16

@shiltian
Copy link
Contributor

I'm working on a patch to add the two variants STRICT_FP_TO_BF16 and STRICT_BF16_TO_FP. For AMDGPU backend, how are we gonna handle the instruction selection? We don't have bfloat related conversion instructions, do we?

@arsenm
Copy link
Contributor Author

arsenm commented Jan 30, 2024

I'm working on a patch to add the two variants STRICT_FP_TO_BF16 and STRICT_BF16_TO_FP. For AMDGPU backend, how are we gonna handle the instruction selection? We don't have bfloat related conversion instructions, do we?

They should get the standard expansions to convert to float

shiltian added a commit to shiltian/llvm-project that referenced this issue Jan 30, 2024
This patch adds the support for `STRICT_BF16_TO_FP` and `STRICT_FP_TO_BF16`.

Fix llvm#78540.
shiltian added a commit to shiltian/llvm-project that referenced this issue Jan 31, 2024
This patch adds the support for `STRICT_BF16_TO_FP` and `STRICT_FP_TO_BF16`.

Fix llvm#78540.
shiltian added a commit to shiltian/llvm-project that referenced this issue Feb 10, 2024
This patch adds the support for `STRICT_BF16_TO_FP` and `STRICT_FP_TO_BF16`.

Fix llvm#78540.
shiltian added a commit to shiltian/llvm-project that referenced this issue Feb 10, 2024
This patch adds the support for `STRICT_BF16_TO_FP` and `STRICT_FP_TO_BF16`.

Fix llvm#78540.
shiltian added a commit to shiltian/llvm-project that referenced this issue Feb 14, 2024
This patch adds the support for `STRICT_BF16_TO_FP` and `STRICT_FP_TO_BF16`.

Fix llvm#78540.
shiltian added a commit to shiltian/llvm-project that referenced this issue Feb 29, 2024
This patch adds the support for `STRICT_BF16_TO_FP` and `STRICT_FP_TO_BF16`.

Fix llvm#78540.
@shiltian
Copy link
Contributor

I'm not sure how to move forward with #80056 because I can't design a valid test case.

@arsenm
Copy link
Contributor Author

arsenm commented Feb 29, 2024

I'm not sure how to move forward with #80056 because I can't design a valid test case.

We can continue to fail to legalize if we know exceptions are enabled

shiltian added a commit to shiltian/llvm-project that referenced this issue Feb 29, 2024
This patch adds the support for `STRICT_BF16_TO_FP` and `STRICT_FP_TO_BF16`.

Fix llvm#78540.
shiltian added a commit to shiltian/llvm-project that referenced this issue Mar 2, 2024
This patch adds the support for `STRICT_BF16_TO_FP` and `STRICT_FP_TO_BF16`.

Fix llvm#78540.
shiltian added a commit to shiltian/llvm-project that referenced this issue Mar 3, 2024
This patch adds the support for `STRICT_BF16_TO_FP` and `STRICT_FP_TO_BF16`.

Fix llvm#78540.
shiltian added a commit to shiltian/llvm-project that referenced this issue Mar 4, 2024
This patch adds the support for `STRICT_BF16_TO_FP` and `STRICT_FP_TO_BF16`.

Fix llvm#78540.
@shiltian
Copy link
Contributor

shiltian commented Mar 4, 2024

This issue has been partially solved by b0c158b. It doesn't hit the assertion now, though we still can't enable those tests because we don't have a valid lowering for AMDGPU.

@shiltian shiltian removed their assignment Feb 6, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend:AMDGPU llvm:SelectionDAG SelectionDAGISel as well
Projects
None yet
Development

No branches or pull requests

3 participants