[Mlir-commits] [mlir] [MLIR][NFC] Speed up is valid symbol check (PR #154924)
William Moses
llvmlistbot at llvm.org
Fri Aug 22 04:53:52 PDT 2025
https://github.com/wsmoses updated https://github.com/llvm/llvm-project/pull/154924
>From 19530291b90a28a9776eacd4369a8d371cb19708 Mon Sep 17 00:00:00 2001
From: "William S. Moses" <gh at wsmoses.com>
Date: Fri, 22 Aug 2025 05:46:29 -0500
Subject: [PATCH 1/5] [MLIR][NFC] Speed up is valid symbol check
---
mlir/lib/Dialect/Affine/IR/AffineOps.cpp | 47 +++++++++++++++---------
1 file changed, 30 insertions(+), 17 deletions(-)
diff --git a/mlir/lib/Dialect/Affine/IR/AffineOps.cpp b/mlir/lib/Dialect/Affine/IR/AffineOps.cpp
index 22608a16cc1ab..8885286f8330a 100644
--- a/mlir/lib/Dialect/Affine/IR/AffineOps.cpp
+++ b/mlir/lib/Dialect/Affine/IR/AffineOps.cpp
@@ -427,6 +427,25 @@ bool mlir::affine::isValidSymbol(Value value) {
return false;
}
+/// A utility function to check if a value is defined at the top level of
+/// `region` or is an argument of `region` or dominates the region.
+static bool isTopLevelValueOrDominator(Value value, Region *region) {
+ Region *parentRegion;
+ if (auto arg = dyn_cast<BlockArgument>(value))
+ parentRegion = arg.getParentRegion();
+ else
+ parentRegion = value.getDefiningOp()->getParentRegion();
+ do {
+ if (parentRegion == region)
+ return true;
+ Operation *regionOp = region->getParentOp();
+ if (regionOp->hasTrait<OpTrait::IsIsolatedFromAbove>())
+ break;
+ region = region->getParentOp()->getParentRegion();
+ } while (region);
+ return false;
+}
+
/// A value can be used as a symbol for `region` iff it meets one of the
/// following conditions:
/// *) It is a constant.
@@ -445,17 +464,11 @@ bool mlir::affine::isValidSymbol(Value value, Region *region) {
return false;
// A top-level value is a valid symbol.
- if (region && ::isTopLevelValue(value, region))
+ if (region && isTopLevelValueOrDominator(value, region))
return true;
auto *defOp = value.getDefiningOp();
if (!defOp) {
- // A block argument that is not a top-level value is a valid symbol if it
- // dominates region's parent op.
- Operation *regionOp = region ? region->getParentOp() : nullptr;
- if (regionOp && !regionOp->hasTrait<OpTrait::IsIsolatedFromAbove>())
- if (auto *parentOpRegion = region->getParentOp()->getParentRegion())
- return isValidSymbol(value, parentOpRegion);
return false;
}
@@ -465,22 +478,22 @@ bool mlir::affine::isValidSymbol(Value value, Region *region) {
return true;
// `Pure` operation that whose operands are valid symbolic identifiers.
- if (isPure(defOp) && llvm::all_of(defOp->getOperands(), [&](Value operand) {
- return affine::isValidSymbol(operand, region);
- })) {
- return true;
+ if (isPure(defOp)) {
+ bool allValid = true;
+ for (auto operand : defOp->getOperands()) {
+ if (!affine::isValidSymbol(operand, region)) {
+ allValid = false;
+ break;
+ }
+ }
+ if (allValid)
+ return true;
}
// Dim op results could be valid symbols at any level.
if (auto dimOp = dyn_cast<ShapedDimOpInterface>(defOp))
return isDimOpValidSymbol(dimOp, region);
- // Check for values dominating `region`'s parent op.
- Operation *regionOp = region ? region->getParentOp() : nullptr;
- if (regionOp && !regionOp->hasTrait<OpTrait::IsIsolatedFromAbove>())
- if (auto *parentRegion = region->getParentOp()->getParentRegion())
- return isValidSymbol(value, parentRegion);
-
return false;
}
>From fadebb7526f4302f2ffcd1de5c9e79d4b63cc3ae Mon Sep 17 00:00:00 2001
From: William Moses <gh at wsmoses.com>
Date: Fri, 22 Aug 2025 13:22:18 +0200
Subject: [PATCH 2/5] Update mlir/lib/Dialect/Affine/IR/AffineOps.cpp
Co-authored-by: Mehdi Amini <joker.eph at gmail.com>
---
mlir/lib/Dialect/Affine/IR/AffineOps.cpp | 6 +-----
1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/mlir/lib/Dialect/Affine/IR/AffineOps.cpp b/mlir/lib/Dialect/Affine/IR/AffineOps.cpp
index 8885286f8330a..aba93901386d6 100644
--- a/mlir/lib/Dialect/Affine/IR/AffineOps.cpp
+++ b/mlir/lib/Dialect/Affine/IR/AffineOps.cpp
@@ -430,11 +430,7 @@ bool mlir::affine::isValidSymbol(Value value) {
/// A utility function to check if a value is defined at the top level of
/// `region` or is an argument of `region` or dominates the region.
static bool isTopLevelValueOrDominator(Value value, Region *region) {
- Region *parentRegion;
- if (auto arg = dyn_cast<BlockArgument>(value))
- parentRegion = arg.getParentRegion();
- else
- parentRegion = value.getDefiningOp()->getParentRegion();
+ Region *parentRegion = value.getParentRegion();
do {
if (parentRegion == region)
return true;
>From 641df18b4c2d226530bf79a3e4133fdd055c3c09 Mon Sep 17 00:00:00 2001
From: "William S. Moses" <gh at wsmoses.com>
Date: Fri, 22 Aug 2025 06:30:40 -0500
Subject: [PATCH 3/5] rename
---
mlir/lib/Dialect/Affine/IR/AffineOps.cpp | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/mlir/lib/Dialect/Affine/IR/AffineOps.cpp b/mlir/lib/Dialect/Affine/IR/AffineOps.cpp
index aba93901386d6..ff4926ea4b8e0 100644
--- a/mlir/lib/Dialect/Affine/IR/AffineOps.cpp
+++ b/mlir/lib/Dialect/Affine/IR/AffineOps.cpp
@@ -428,8 +428,8 @@ bool mlir::affine::isValidSymbol(Value value) {
}
/// A utility function to check if a value is defined at the top level of
-/// `region` or is an argument of `region` or dominates the region.
-static bool isTopLevelValueOrDominator(Value value, Region *region) {
+/// `region` or is an argument of `region` or is defined above the region.
+static bool isTopLevelValueOrAbove(Value value, Region *region) {
Region *parentRegion = value.getParentRegion();
do {
if (parentRegion == region)
@@ -460,7 +460,7 @@ bool mlir::affine::isValidSymbol(Value value, Region *region) {
return false;
// A top-level value is a valid symbol.
- if (region && isTopLevelValueOrDominator(value, region))
+ if (region && isTopLevelValueOrAbove(value, region))
return true;
auto *defOp = value.getDefiningOp();
>From d1600dd4652f276230c3531204852feeff976ea5 Mon Sep 17 00:00:00 2001
From: "William S. Moses" <gh at wsmoses.com>
Date: Fri, 22 Aug 2025 06:32:28 -0500
Subject: [PATCH 4/5] nit
---
mlir/lib/Dialect/Affine/IR/AffineOps.cpp | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/mlir/lib/Dialect/Affine/IR/AffineOps.cpp b/mlir/lib/Dialect/Affine/IR/AffineOps.cpp
index ff4926ea4b8e0..4ac86561b4f27 100644
--- a/mlir/lib/Dialect/Affine/IR/AffineOps.cpp
+++ b/mlir/lib/Dialect/Affine/IR/AffineOps.cpp
@@ -464,9 +464,8 @@ bool mlir::affine::isValidSymbol(Value value, Region *region) {
return true;
auto *defOp = value.getDefiningOp();
- if (!defOp) {
+ if (!defOp)
return false;
- }
// Constant operation is ok.
Attribute operandCst;
>From 0e07cb92c7e499b2c7f8c090148f729d6462f574 Mon Sep 17 00:00:00 2001
From: "William S. Moses" <gh at wsmoses.com>
Date: Fri, 22 Aug 2025 06:53:37 -0500
Subject: [PATCH 5/5] fix
---
mlir/lib/Dialect/Affine/IR/AffineOps.cpp | 14 ++++----------
1 file changed, 4 insertions(+), 10 deletions(-)
diff --git a/mlir/lib/Dialect/Affine/IR/AffineOps.cpp b/mlir/lib/Dialect/Affine/IR/AffineOps.cpp
index 4ac86561b4f27..7e5ce26b5f733 100644
--- a/mlir/lib/Dialect/Affine/IR/AffineOps.cpp
+++ b/mlir/lib/Dialect/Affine/IR/AffineOps.cpp
@@ -473,16 +473,10 @@ bool mlir::affine::isValidSymbol(Value value, Region *region) {
return true;
// `Pure` operation that whose operands are valid symbolic identifiers.
- if (isPure(defOp)) {
- bool allValid = true;
- for (auto operand : defOp->getOperands()) {
- if (!affine::isValidSymbol(operand, region)) {
- allValid = false;
- break;
- }
- }
- if (allValid)
- return true;
+ if (isPure(defOp) && llvm::all_of(defOp->getOperands(), [&](Value operand) {
+ return affine::isValidSymbol(operand, region);
+ })) {
+ return true;
}
// Dim op results could be valid symbols at any level.
More information about the Mlir-commits
mailing list