Skip to content
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

[AMDGPU][GlobalISel] Allow bitcast of bf16 #81674

Closed
wants to merge 1 commit into from

Conversation

shiltian
Copy link
Contributor

@shiltian shiltian commented Feb 13, 2024

This is a prime patch for #80908. This change doesn't break anything, though we can't test that much, given what mentioned in https://github.com/llvm/llvm-project/pull/80908/files#r1483394728.

@llvmbot
Copy link
Collaborator

llvmbot commented Feb 13, 2024

@llvm/pr-subscribers-llvm-globalisel

Author: Shilei Tian (shiltian)

Changes

Full diff: https://github.com/llvm/llvm-project/pull/81674.diff

1 Files Affected:

  • (modified) llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp (+3-2)
diff --git a/llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp b/llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp
index 311dd9d9739a6d..3290262816ef0a 100644
--- a/llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp
+++ b/llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp
@@ -1562,8 +1562,9 @@ bool IRTranslator::translateBitCast(const User &U,
 
 bool IRTranslator::translateCast(unsigned Opcode, const User &U,
                                  MachineIRBuilder &MIRBuilder) {
-  if (U.getType()->getScalarType()->isBFloatTy() ||
-      U.getOperand(0)->getType()->getScalarType()->isBFloatTy())
+  if (Opcode != TargetOpcode::G_BITCAST &&
+      (U.getType()->getScalarType()->isBFloatTy() ||
+       U.getOperand(0)->getType()->getScalarType()->isBFloatTy()))
     return false;
   Register Op = getOrCreateVReg(*U.getOperand(0));
   Register Res = getOrCreateVReg(U);

Copy link
Collaborator

@rampitec rampitec left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is there for a reason. You cannot really enable it without implementing global isel support for bf16. It will start failing silently. The point of this code to fail with an error. It will also start silently failing for multiple targets.

@shiltian shiltian closed this Feb 13, 2024
@shiltian
Copy link
Contributor Author

It is there for a reason. You cannot really enable it without implementing global isel support for bf16. It will start failing silently. The point of this code to fail with an error. It will also start silently failing for multiple targets.

Okay. Then I'll disable the test in #80908 since it is not supported yet.

@arsenm
Copy link
Contributor

arsenm commented Feb 14, 2024

It is there for a reason. You cannot really enable it without implementing global isel support for bf16. It will start failing silently. The point of this code to fail with an error. It will also start silently failing for multiple targets.

You can, bitcast doesn't count as a bfloat operation. As long as we fail on all the actual bfloat operations, it's OK.

@shiltian shiltian deleted the allow-bitcast-bf16 branch February 14, 2024 18:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants