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

[mlir][nfc] De-duplicate tests from Type::isIntOrFloat #129710

Merged
merged 1 commit into from
Mar 6, 2025

Conversation

banach-space
Copy link
Contributor

This PR makes sure that we always use Type::isIntOrFloat rather than
re-implementing this condition inline. Also, it removes isScalarType
that effectively re-implemented this method.

This PR makes sure that we always use `Type::isIntOrFloat` rather than
re-implementing this condition inline. Also, it removes `isScalarType`
that effectively re-implemented this method.
@banach-space banach-space requested review from gysit and ftynse and removed request for gysit March 4, 2025 14:08
@llvmbot
Copy link
Member

llvmbot commented Mar 4, 2025

@llvm/pr-subscribers-mlir-llvm

@llvm/pr-subscribers-mlir

Author: Andrzej Warzyński (banach-space)

Changes

This PR makes sure that we always use Type::isIntOrFloat rather than
re-implementing this condition inline. Also, it removes isScalarType
that effectively re-implemented this method.


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

2 Files Affected:

  • (modified) mlir/lib/Interfaces/DataLayoutInterfaces.cpp (+2-2)
  • (modified) mlir/lib/Target/LLVMIR/ModuleImport.cpp (+3-8)
diff --git a/mlir/lib/Interfaces/DataLayoutInterfaces.cpp b/mlir/lib/Interfaces/DataLayoutInterfaces.cpp
index 049d7f123cec8..3c231d55dfa71 100644
--- a/mlir/lib/Interfaces/DataLayoutInterfaces.cpp
+++ b/mlir/lib/Interfaces/DataLayoutInterfaces.cpp
@@ -51,7 +51,7 @@ mlir::detail::getDefaultTypeSize(Type type, const DataLayout &dataLayout,
 llvm::TypeSize
 mlir::detail::getDefaultTypeSizeInBits(Type type, const DataLayout &dataLayout,
                                        DataLayoutEntryListRef params) {
-  if (isa<IntegerType, FloatType>(type))
+  if (type.isIntOrFloat())
     return llvm::TypeSize::getFixed(type.getIntOrFloatBitWidth());
 
   if (auto ctype = dyn_cast<ComplexType>(type)) {
@@ -720,7 +720,7 @@ LogicalResult mlir::detail::verifyDataLayoutSpec(DataLayoutSpecInterface spec,
       continue;
     }
 
-    if (isa<IntegerType, FloatType>(sampleType)) {
+    if (sampleType.isIntOrFloat()) {
       for (DataLayoutEntryInterface entry : kvp.second) {
         auto value = dyn_cast<DenseIntElementsAttr>(entry.getValue());
         if (!value || !value.getElementType().isSignlessInteger(64)) {
diff --git a/mlir/lib/Target/LLVMIR/ModuleImport.cpp b/mlir/lib/Target/LLVMIR/ModuleImport.cpp
index 8445e609c2244..01749bafd7de3 100644
--- a/mlir/lib/Target/LLVMIR/ModuleImport.cpp
+++ b/mlir/lib/Target/LLVMIR/ModuleImport.cpp
@@ -759,11 +759,6 @@ void ModuleImport::setFastmathFlagsAttr(llvm::Instruction *inst,
   iface->setAttr(iface.getFastmathAttrName(), attr);
 }
 
-/// Returns if `type` is a scalar integer or floating-point type.
-static bool isScalarType(Type type) {
-  return isa<IntegerType, FloatType>(type);
-}
-
 /// Returns `type` if it is a builtin integer or floating-point vector type that
 /// can be used to create an attribute or nullptr otherwise. If provided,
 /// `arrayShape` is added to the shape of the vector to create an attribute that
@@ -781,7 +776,7 @@ static Type getVectorTypeForAttr(Type type, ArrayRef<int64_t> arrayShape = {}) {
 
   // An LLVM dialect vector can only contain scalars.
   Type elementType = LLVM::getVectorElementType(type);
-  if (!isScalarType(elementType))
+  if (!elementType.isIntOrFloat())
     return {};
 
   SmallVector<int64_t> shape(arrayShape);
@@ -794,7 +789,7 @@ Type ModuleImport::getBuiltinTypeForAttr(Type type) {
     return {};
 
   // Return builtin integer and floating-point types as is.
-  if (isScalarType(type))
+  if (type.isIntOrFloat())
     return type;
 
   // Return builtin vectors of integer and floating-point types as is.
@@ -808,7 +803,7 @@ Type ModuleImport::getBuiltinTypeForAttr(Type type) {
     arrayShape.push_back(arrayType.getNumElements());
     type = arrayType.getElementType();
   }
-  if (isScalarType(type))
+  if (type.isIntOrFloat())
     return RankedTensorType::get(arrayShape, type);
   return getVectorTypeForAttr(type, arrayShape);
 }

Copy link
Contributor

@javedabsar1 javedabsar1 left a comment

Choose a reason for hiding this comment

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

LGTM as

bool Type::isIntOrFloat() const {
  return llvm::isa<IntegerType, FloatType>(*this);
}
```

@banach-space banach-space merged commit 620c383 into llvm:main Mar 6, 2025
14 checks passed
jph-13 pushed a commit to jph-13/llvm-project that referenced this pull request Mar 21, 2025
This PR makes sure that we always use `Type::isIntOrFloat` rather than
re-implementing this condition inline. Also, it removes `isScalarType`
that effectively re-implemented this method.
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.

3 participants