-
Notifications
You must be signed in to change notification settings - Fork 11.9k
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
[MLIR] [NFC] Use APFloat semantics to get floating type width #107372
Conversation
@llvm/pr-subscribers-mlir-core @llvm/pr-subscribers-mlir Author: Sergey Kozub (sergey-kozub) ChangesAs suggested in the comments of #105573 Full diff: https://github.com/llvm/llvm-project/pull/107372.diff 1 Files Affected:
diff --git a/mlir/lib/IR/BuiltinTypes.cpp b/mlir/lib/IR/BuiltinTypes.cpp
index 16b53efa55fb80..58f83333b00c20 100644
--- a/mlir/lib/IR/BuiltinTypes.cpp
+++ b/mlir/lib/IR/BuiltinTypes.cpp
@@ -91,21 +91,9 @@ IntegerType IntegerType::scaleElementBitwidth(unsigned scale) {
//===----------------------------------------------------------------------===//
unsigned FloatType::getWidth() {
- if (llvm::isa<Float8E5M2Type, Float8E4M3Type, Float8E4M3FNType,
- Float8E5M2FNUZType, Float8E4M3FNUZType, Float8E4M3B11FNUZType,
- Float8E3M4Type>(*this))
- return 8;
- if (llvm::isa<Float16Type, BFloat16Type>(*this))
- return 16;
- if (llvm::isa<Float32Type, FloatTF32Type>(*this))
+ if (llvm::isa<FloatTF32Type>(*this))
return 32;
- if (llvm::isa<Float64Type>(*this))
- return 64;
- if (llvm::isa<Float80Type>(*this))
- return 80;
- if (llvm::isa<Float128Type>(*this))
- return 128;
- llvm_unreachable("unexpected float type");
+ return getFloatSemantics().sizeInBits;
}
/// Returns the floating semantics for the given type.
|
7102cae
to
ad2cce9
Compare
if (llvm::isa<Float16Type, BFloat16Type>(*this)) | ||
return 16; | ||
if (llvm::isa<Float32Type, FloatTF32Type>(*this)) | ||
if (llvm::isa<FloatTF32Type>(*this)) | ||
return 32; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added the comment, thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems fine I think, this is NFC right? (please edit the PR title)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I don't have a problem with this either
ad2cce9
to
bb4556d
Compare
…07372) As suggested in the comments of llvm#105573
As suggested in the comments of #105573