Skip to content

Conversation

@CoTinker
Copy link
Contributor

@CoTinker CoTinker commented Oct 18, 2024

This PR adds an isFloat() function without checking width and refactors the code for clarity and maintainability.

@llvmbot llvmbot added mlir:core MLIR Core Infrastructure mlir labels Oct 18, 2024
@CoTinker CoTinker requested a review from jpienaar October 18, 2024 12:35
@llvmbot
Copy link
Member

llvmbot commented Oct 18, 2024

@llvm/pr-subscribers-mlir

@llvm/pr-subscribers-mlir-core

Author: Longsheng Mou (CoTinker)

Changes

This PR adds an isFloat() function without checking width and refactors the code for clarity and maintainability.


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

2 Files Affected:

  • (modified) mlir/include/mlir/IR/Types.h (+3-2)
  • (modified) mlir/lib/IR/Types.cpp (+12-8)
diff --git a/mlir/include/mlir/IR/Types.h b/mlir/include/mlir/IR/Types.h
index acd0f894abbbe6..30e151ece4850c 100644
--- a/mlir/include/mlir/IR/Types.h
+++ b/mlir/include/mlir/IR/Types.h
@@ -125,6 +125,7 @@ class Type {
   // Convenience predicates.  This is only for floating point types,
   // derived types should use isa/dyn_cast.
   bool isIndex() const;
+  bool isFloat() const;
   bool isFloat4E2M1FN() const;
   bool isFloat6E2M3FN() const;
   bool isFloat6E3M2FN() const;
@@ -164,10 +165,10 @@ class Type {
 
   /// Return true if this is a signless integer or index type.
   bool isSignlessIntOrIndex() const;
-  /// Return true if this is a signless integer, index, or float type.
-  bool isSignlessIntOrIndexOrFloat() const;
   /// Return true of this is a signless integer or a float type.
   bool isSignlessIntOrFloat() const;
+  /// Return true if this is a signless integer, index, or float type.
+  bool isSignlessIntOrIndexOrFloat() const;
 
   /// Return true if this is an integer (of any signedness) or an index type.
   bool isIntOrIndex() const;
diff --git a/mlir/lib/IR/Types.cpp b/mlir/lib/IR/Types.cpp
index e190902b2e4898..d45d21af2197d9 100644
--- a/mlir/lib/IR/Types.cpp
+++ b/mlir/lib/IR/Types.cpp
@@ -63,6 +63,8 @@ bool Type::isF128() const { return llvm::isa<Float128Type>(*this); }
 
 bool Type::isIndex() const { return llvm::isa<IndexType>(*this); }
 
+bool Type::isFloat() const { return llvm::isa<FloatType>(*this); }
+
 bool Type::isInteger() const { return llvm::isa<IntegerType>(*this); }
 
 /// Return true if this is an integer type with the specified width.
@@ -109,26 +111,28 @@ bool Type::isUnsignedInteger(unsigned width) const {
 }
 
 bool Type::isSignlessIntOrIndex() const {
-  return isSignlessInteger() || llvm::isa<IndexType>(*this);
+  return isSignlessInteger() || isIndex();
 }
 
-bool Type::isSignlessIntOrIndexOrFloat() const {
-  return isSignlessInteger() || llvm::isa<IndexType, FloatType>(*this);
+bool Type::isSignlessIntOrFloat() const {
+  return isSignlessInteger() || isFloat();
 }
 
-bool Type::isSignlessIntOrFloat() const {
-  return isSignlessInteger() || llvm::isa<FloatType>(*this);
+bool Type::isSignlessIntOrIndexOrFloat() const {
+  return isSignlessIntOrIndex() || isFloat();
 }
 
 bool Type::isIntOrIndex() const {
-  return llvm::isa<IntegerType>(*this) || isIndex();
+  return isInteger() || isIndex();
 }
 
 bool Type::isIntOrFloat() const {
-  return llvm::isa<IntegerType, FloatType>(*this);
+  return isInteger() || isFloat();
 }
 
-bool Type::isIntOrIndexOrFloat() const { return isIntOrFloat() || isIndex(); }
+bool Type::isIntOrIndexOrFloat() const {
+  return isIntOrIndex() || isFloat();
+}
 
 unsigned Type::getIntOrFloatBitWidth() const {
   assert(isIntOrFloat() && "only integers and floats have a bitwidth");

This PR adds an isFloat() function without checking width and
refactors the code for clarity and maintainability.
@CoTinker
Copy link
Contributor Author

Ping~

bool Type::isIndex() const { return llvm::isa<IndexType>(*this); }

bool Type::isFloat() const { return llvm::isa<FloatType>(*this); }

Copy link
Collaborator

Choose a reason for hiding this comment

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

This can be seen as fitting here for consistency, however I wonder if the desirable thing wouldn't instead be to remove all the other isXXXX from the generic Type class?

@joker-eph
Copy link
Collaborator

Seems like a duplicate from #88926

@CoTinker
Copy link
Contributor Author

Okay

@CoTinker CoTinker closed this Oct 21, 2024
@CoTinker CoTinker deleted the isfloat branch October 22, 2024 00:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

mlir:core MLIR Core Infrastructure mlir

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants