[MLIR][Types] add isFloat() to Type class

[rscottmanley]

Add isFloat() as a member function to the Type class and canonicalize the compound queries to use the base queries.

[github-actions]

Thank you for submitting a Pull Request (PR) to the LLVM Project!

This PR will be automatically labeled and the relevant teams will be
notified.

If you wish to, you can add reviewers by using the "Reviewers" section on this page.

If this is not working for you, it is probably because you do not have write
permissions for the repository. In which case you can instead tag reviewers by
name in a comment by using @ followed by their GitHub username.

If you have received no comments on your PR for a week, you can request a review
by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate
is once a week. Please remember that you are asking for valuable time from other developers.

If you have further questions, they may be answered by the LLVM GitHub User Guide.

You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums.

[llvmbot]

@llvm/pr-subscribers-mlir-core

@llvm/pr-subscribers-mlir

Author: Scott Manley (rscottmanley)

Changes

Add isFloat() as a member function to the Type class and canonicalize the compound queries to use the base queries.

---

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

2 Files Affected:

• (modified) mlir/include/mlir/IR/Types.h (+4-1)
• (modified) mlir/lib/IR/Types.cpp (+7-5)

diff --git a/mlir/include/mlir/IR/Types.h b/mlir/include/mlir/IR/Types.h
index a89e13b625bf40..47306fbac6e681 100644
--- a/mlir/include/mlir/IR/Types.h
+++ b/mlir/include/mlir/IR/Types.h
@@ -133,7 +133,10 @@ class Type {
   bool isF80() const;
   bool isF128() const;
 
-  /// Return true if this is an integer type (with the specified width).
+  /// Return true if this is a floating point type.
+  bool isFloat() const;
+
+  /// Return true if this is an integer type.
   bool isInteger() const;
   bool isInteger(unsigned width) const;
   /// Return true if this is a signless integer type (with the specified width).
diff --git a/mlir/lib/IR/Types.cpp b/mlir/lib/IR/Types.cpp
index 1d1ba6df4db2f7..a18ef525143aed 100644
--- a/mlir/lib/IR/Types.cpp
+++ b/mlir/lib/IR/Types.cpp
@@ -53,6 +53,8 @@ bool Type::isF64() const { return llvm::isa<Float64Type>(*this); }
 bool Type::isF80() const { return llvm::isa<Float80Type>(*this); }
 bool Type::isF128() const { return llvm::isa<Float128Type>(*this); }
 
+bool Type::isFloat() const { return llvm::isa<FloatType>(*this); }
+
 bool Type::isIndex() const { return llvm::isa<IndexType>(*this); }
 
 bool Type::isInteger() const { return llvm::isa<IntegerType>(*this); }
@@ -101,23 +103,23 @@ 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);
+  return isSignlessInteger() || isIndex() || isFloat();
 }
 
 bool Type::isSignlessIntOrFloat() const {
-  return isSignlessInteger() || llvm::isa<FloatType>(*this);
+  return isSignlessInteger() || 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(); }

[github-actions]

✅ With the latest revision this PR passed the C/C++ code formatter.

[jpienaar]

I'm not convinced float should be privileged like this. This also introduces a different way to query the same info as already have via the isa/dyn_cast etc queries.

[rscottmanley]

I'm not convinced float should be privileged like this. This also introduces a different way to query the same info as already have via the isa/dyn_cast etc queries.

Are you saying none of the current queries in the Type class should exist? All that's really being done here is adding a specific function for isFloat() that's already implied by the existing isSignlessIntOrIndexOrFloat() or isIntOrFloat() etc

[rscottmanley]

ping -- if we don't want to allow these, should we actively rip out the others? If we think the rest should remain, I don't see a reason to not allow my change.

[ftynse]

FWIW, I do think we should remove the others. They are a historic artifact of there not being specific classes for subclasses of FloatType that we could use with isa. Having only one way to check for subclass relation looks saner. But it's a breaking change...

I suppose @joker-eph will have an opinion as well.

[joker-eph]

I agree with @ftynse (and @jpienaar) here.