Skip to content

[mlir][IR] Add isFloat() and clean code #112897

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

Closed
wants to merge 1 commit into from
Closed

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~

@@ -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); }

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