summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorBilly Zhu <billyzhu@modular.com>2023-12-21 09:54:48 -0800
committerGitHub <noreply@github.com>2023-12-21 09:54:48 -0800
commit34a65980d7d2e1b05e3fc88535cafe606ee55e04 (patch)
tree8cd0159f9c84b80516276d2ad98fff09cea9546e
parenta4e15416b41459b6f69086a22088520ee826f244 (diff)
[MLIR] Erase location of folded constants (#75415)
Follow up to the discussion from #75258, and serves as an alternate solution for #74670. Set the location to Unknown for deduplicated / moved / materialized constants by OperationFolder. This makes sure that the folded constants don't end up with an arbitrary location of one of the original ops that became it, and that hoisted ops don't confuse the stepping order.
-rw-r--r--mlir/include/mlir/Transforms/FoldUtils.h8
-rw-r--r--mlir/lib/Transforms/SCCP.cpp2
-rw-r--r--mlir/lib/Transforms/Utils/FoldUtils.cpp33
-rw-r--r--mlir/test/Dialect/Transform/test-pattern-application.mlir2
-rw-r--r--mlir/test/Transforms/canonicalize-debuginfo.mlir46
-rw-r--r--mlir/test/Transforms/constant-fold-debuginfo.mlir42
-rw-r--r--mlir/test/lib/Transforms/TestIntRangeInference.cpp5
7 files changed, 120 insertions, 18 deletions
diff --git a/mlir/include/mlir/Transforms/FoldUtils.h b/mlir/include/mlir/Transforms/FoldUtils.h
index 2600da361496..2e7a6fe3e362 100644
--- a/mlir/include/mlir/Transforms/FoldUtils.h
+++ b/mlir/include/mlir/Transforms/FoldUtils.h
@@ -33,7 +33,8 @@ class Value;
class OperationFolder {
public:
OperationFolder(MLIRContext *ctx, OpBuilder::Listener *listener = nullptr)
- : interfaces(ctx), rewriter(ctx, listener) {}
+ : erasedFoldedLocation(UnknownLoc::get(ctx)), interfaces(ctx),
+ rewriter(ctx, listener) {}
/// Tries to perform folding on the given `op`, including unifying
/// deduplicated constants. If successful, replaces `op`'s uses with
@@ -65,7 +66,7 @@ public:
/// be created in a parent block. On success this returns the constant
/// operation, nullptr otherwise.
Value getOrCreateConstant(Block *block, Dialect *dialect, Attribute value,
- Type type, Location loc);
+ Type type);
private:
/// This map keeps track of uniqued constants by dialect, attribute, and type.
@@ -95,6 +96,9 @@ private:
Dialect *dialect, Attribute value,
Type type, Location loc);
+ /// The location to overwrite with for folder-owned constants.
+ UnknownLoc erasedFoldedLocation;
+
/// A mapping between an insertion region and the constants that have been
/// created within it.
DenseMap<Region *, ConstantMap> foldScopes;
diff --git a/mlir/lib/Transforms/SCCP.cpp b/mlir/lib/Transforms/SCCP.cpp
index 14435b37acc9..b2d3929b0459 100644
--- a/mlir/lib/Transforms/SCCP.cpp
+++ b/mlir/lib/Transforms/SCCP.cpp
@@ -53,7 +53,7 @@ static LogicalResult replaceWithConstant(DataFlowSolver &solver,
Dialect *dialect = latticeValue.getConstantDialect();
Value constant = folder.getOrCreateConstant(
builder.getInsertionBlock(), dialect, latticeValue.getConstantValue(),
- value.getType(), value.getLoc());
+ value.getType());
if (!constant)
return failure();
diff --git a/mlir/lib/Transforms/Utils/FoldUtils.cpp b/mlir/lib/Transforms/Utils/FoldUtils.cpp
index eb4dcb251a22..e5f78abf7fca 100644
--- a/mlir/lib/Transforms/Utils/FoldUtils.cpp
+++ b/mlir/lib/Transforms/Utils/FoldUtils.cpp
@@ -77,8 +77,10 @@ LogicalResult OperationFolder::tryToFold(Operation *op, bool *inPlaceUpdate) {
// Check to see if we should rehoist, i.e. if a non-constant operation was
// inserted before this one.
Block *opBlock = op->getBlock();
- if (&opBlock->front() != op && !isFolderOwnedConstant(op->getPrevNode()))
+ if (&opBlock->front() != op && !isFolderOwnedConstant(op->getPrevNode())) {
op->moveBefore(&opBlock->front());
+ op->setLoc(erasedFoldedLocation);
+ }
return failure();
}
@@ -112,8 +114,10 @@ bool OperationFolder::insertKnownConstant(Operation *op, Attribute constValue) {
// If this is a constant we unique'd, we don't need to insert, but we can
// check to see if we should rehoist it.
if (isFolderOwnedConstant(op)) {
- if (&opBlock->front() != op && !isFolderOwnedConstant(op->getPrevNode()))
+ if (&opBlock->front() != op && !isFolderOwnedConstant(op->getPrevNode())) {
op->moveBefore(&opBlock->front());
+ op->setLoc(erasedFoldedLocation);
+ }
return true;
}
@@ -142,6 +146,7 @@ bool OperationFolder::insertKnownConstant(Operation *op, Attribute constValue) {
if (folderConstOp) {
notifyRemoval(op);
rewriter.replaceOp(op, folderConstOp->getResults());
+ folderConstOp->setLoc(erasedFoldedLocation);
return false;
}
@@ -151,8 +156,10 @@ bool OperationFolder::insertKnownConstant(Operation *op, Attribute constValue) {
// anything. Otherwise, we move the constant to the insertion block.
Block *insertBlock = &insertRegion->front();
if (opBlock != insertBlock || (&insertBlock->front() != op &&
- !isFolderOwnedConstant(op->getPrevNode())))
+ !isFolderOwnedConstant(op->getPrevNode()))) {
op->moveBefore(&insertBlock->front());
+ op->setLoc(erasedFoldedLocation);
+ }
folderConstOp = op;
referencedDialects[op].push_back(op->getDialect());
@@ -193,17 +200,17 @@ void OperationFolder::clear() {
/// Get or create a constant using the given builder. On success this returns
/// the constant operation, nullptr otherwise.
Value OperationFolder::getOrCreateConstant(Block *block, Dialect *dialect,
- Attribute value, Type type,
- Location loc) {
+ Attribute value, Type type) {
// Find an insertion point for the constant.
auto *insertRegion = getInsertionRegion(interfaces, block);
auto &entry = insertRegion->front();
rewriter.setInsertionPoint(&entry, entry.begin());
// Get the constant map for the insertion region of this operation.
+ // Use erased location since the op is being built at the front of block.
auto &uniquedConstants = foldScopes[insertRegion];
- Operation *constOp =
- tryGetOrCreateConstant(uniquedConstants, dialect, value, type, loc);
+ Operation *constOp = tryGetOrCreateConstant(uniquedConstants, dialect, value,
+ type, erasedFoldedLocation);
return constOp ? constOp->getResult(0) : Value();
}
@@ -254,8 +261,9 @@ OperationFolder::processFoldResults(Operation *op,
// Check to see if there is a canonicalized version of this constant.
auto res = op->getResult(i);
Attribute attrRepl = foldResults[i].get<Attribute>();
- if (auto *constOp = tryGetOrCreateConstant(
- uniquedConstants, dialect, attrRepl, res.getType(), op->getLoc())) {
+ if (auto *constOp =
+ tryGetOrCreateConstant(uniquedConstants, dialect, attrRepl,
+ res.getType(), erasedFoldedLocation)) {
// Ensure that this constant dominates the operation we are replacing it
// with. This may not automatically happen if the operation being folded
// was inserted before the constant within the insertion block.
@@ -290,8 +298,11 @@ OperationFolder::tryGetOrCreateConstant(ConstantMap &uniquedConstants,
// Check if an existing mapping already exists.
auto constKey = std::make_tuple(dialect, value, type);
Operation *&constOp = uniquedConstants[constKey];
- if (constOp)
+ if (constOp) {
+ if (loc != constOp->getLoc())
+ constOp->setLoc(erasedFoldedLocation);
return constOp;
+ }
// If one doesn't exist, try to materialize one.
if (!(constOp = materializeConstant(dialect, rewriter, value, type, loc)))
@@ -314,6 +325,8 @@ OperationFolder::tryGetOrCreateConstant(ConstantMap &uniquedConstants,
notifyRemoval(constOp);
rewriter.eraseOp(constOp);
referencedDialects[existingOp].push_back(dialect);
+ if (loc != existingOp->getLoc())
+ existingOp->setLoc(erasedFoldedLocation);
return constOp = existingOp;
}
diff --git a/mlir/test/Dialect/Transform/test-pattern-application.mlir b/mlir/test/Dialect/Transform/test-pattern-application.mlir
index 2fd47c6bae39..ff9a535c8384 100644
--- a/mlir/test/Dialect/Transform/test-pattern-application.mlir
+++ b/mlir/test/Dialect/Transform/test-pattern-application.mlir
@@ -179,7 +179,6 @@ module {
// CHECK: return %[[c5]]
func.func @canonicalization(%t: tensor<5xf32>) -> index {
%c0 = arith.constant 0 : index
- // expected-remark @below {{op was replaced}}
%dim = tensor.dim %t, %c0 : tensor<5xf32>
return %dim : index
}
@@ -191,7 +190,6 @@ transform.sequence failures(propagate) {
transform.apply_patterns to %1 {
transform.apply_patterns.canonicalization
} : !transform.any_op
- transform.test_print_remark_at_operand %0, "op was replaced" : !transform.any_op
}
// -----
diff --git a/mlir/test/Transforms/canonicalize-debuginfo.mlir b/mlir/test/Transforms/canonicalize-debuginfo.mlir
new file mode 100644
index 000000000000..30c8022daa76
--- /dev/null
+++ b/mlir/test/Transforms/canonicalize-debuginfo.mlir
@@ -0,0 +1,46 @@
+// RUN: mlir-opt %s -pass-pipeline='builtin.module(func.func(canonicalize{test-convergence}))' -split-input-file -mlir-print-debuginfo | FileCheck %s
+
+// CHECK-LABEL: func @merge_constants
+func.func @merge_constants() -> (index, index, index, index) {
+ // CHECK-NEXT: arith.constant 42 : index loc(#[[UnknownLoc:.*]])
+ %0 = arith.constant 42 : index loc("merge_constants":0:0)
+ %1 = arith.constant 42 : index loc("merge_constants":1:0)
+ %2 = arith.constant 42 : index loc("merge_constants":2:0)
+ %3 = arith.constant 42 : index loc("merge_constants":2:0)
+ return %0, %1, %2, %3 : index, index, index, index
+}
+// CHECK: #[[UnknownLoc]] = loc(unknown)
+
+// -----
+
+// CHECK-LABEL: func @simple_hoist
+func.func @simple_hoist(%arg0: memref<8xi32>) -> i32 {
+ // CHECK: arith.constant 88 : i32 loc(#[[UnknownLoc:.*]])
+ // CHECK: arith.constant 42 : i32 loc(#[[ConstLoc0:.*]])
+ // CHECK: arith.constant 0 : index loc(#[[ConstLoc1:.*]])
+ %0 = arith.constant 42 : i32 loc("simple_hoist":0:0)
+ %1 = arith.constant 0 : index loc("simple_hoist":1:0)
+ memref.store %0, %arg0[%1] : memref<8xi32>
+
+ %2 = arith.constant 88 : i32 loc("simple_hoist":2:0)
+
+ return %2 : i32
+}
+// CHECK-DAG: #[[UnknownLoc]] = loc(unknown)
+// CHECK-DAG: #[[ConstLoc0]] = loc("simple_hoist":0:0)
+// CHECK-DAG: #[[ConstLoc1]] = loc("simple_hoist":1:0)
+
+// -----
+
+// CHECK-LABEL: func @hoist_and_merge
+func.func @hoist_and_merge(%arg0: memref<8xi32>) {
+ // CHECK-NEXT: arith.constant 42 : i32 loc(#[[UnknownLoc:.*]])
+ affine.for %arg1 = 0 to 8 {
+ %0 = arith.constant 42 : i32 loc("hoist_and_merge":0:0)
+ %1 = arith.constant 42 : i32 loc("hoist_and_merge":1:0)
+ memref.store %0, %arg0[%arg1] : memref<8xi32>
+ memref.store %1, %arg0[%arg1] : memref<8xi32>
+ }
+ return
+} loc("hoist_and_merge":2:0)
+// CHECK: #[[UnknownLoc]] = loc(unknown)
diff --git a/mlir/test/Transforms/constant-fold-debuginfo.mlir b/mlir/test/Transforms/constant-fold-debuginfo.mlir
new file mode 100644
index 000000000000..c308bc477bee
--- /dev/null
+++ b/mlir/test/Transforms/constant-fold-debuginfo.mlir
@@ -0,0 +1,42 @@
+// RUN: mlir-opt %s -split-input-file -test-constant-fold -mlir-print-debuginfo | FileCheck %s
+
+// CHECK-LABEL: func @fold_and_merge
+func.func @fold_and_merge() -> (i32, i32) {
+ // CHECK-NEXT: [[C:%.+]] = arith.constant 6 : i32 loc(#[[UnknownLoc:.*]])
+ %0 = arith.constant 1 : i32 loc("fold_and_merge":0:0)
+ %1 = arith.constant 5 : i32 loc("fold_and_merge":1:0)
+ %2 = arith.addi %0, %1 : i32 loc("fold_and_merge":2:0)
+
+ %3 = arith.constant 6 : i32 loc("fold_and_merge":3:0)
+
+ return %2, %3: i32, i32
+}
+// CHECK: #[[UnknownLoc]] = loc(unknown)
+
+// -----
+
+// CHECK-LABEL: func @materialize_different_dialect
+func.func @materialize_different_dialect() -> (f32, f32) {
+ // CHECK: arith.constant 1.{{0*}}e+00 : f32 loc(#[[UnknownLoc:.*]])
+ %0 = arith.constant -1.0 : f32 loc("materialize_different_dialect":0:0)
+ %1 = math.absf %0 : f32 loc("materialize_different_dialect":1:0)
+ %2 = arith.constant 1.0 : f32 loc("materialize_different_dialect":2:0)
+
+ return %1, %2: f32, f32
+}
+// CHECK: #[[UnknownLoc]] = loc(unknown)
+
+// -----
+
+// CHECK-LABEL: func @materialize_in_front
+func.func @materialize_in_front(%arg0: memref<8xi32>) {
+ // CHECK-NEXT: arith.constant 6 : i32 loc(#[[UnknownLoc:.*]])
+ affine.for %arg1 = 0 to 8 {
+ %1 = arith.constant 1 : i32 loc("materialize_in_front":0:0)
+ %2 = arith.constant 5 : i32 loc("materialize_in_front":1:0)
+ %3 = arith.addi %1, %2 : i32 loc("materialize_in_front":2:0)
+ memref.store %3, %arg0[%arg1] : memref<8xi32>
+ }
+ return
+} loc("materialize_in_front":3:0)
+// CHECK: #[[UnknownLoc]] = loc(unknown)
diff --git a/mlir/test/lib/Transforms/TestIntRangeInference.cpp b/mlir/test/lib/Transforms/TestIntRangeInference.cpp
index 2f6dd5b8095d..5758f6acf2f0 100644
--- a/mlir/test/lib/Transforms/TestIntRangeInference.cpp
+++ b/mlir/test/lib/Transforms/TestIntRangeInference.cpp
@@ -40,9 +40,8 @@ static LogicalResult replaceWithConstant(DataFlowSolver &solver, OpBuilder &b,
maybeDefiningOp ? maybeDefiningOp->getDialect()
: value.getParentRegion()->getParentOp()->getDialect();
Attribute constAttr = b.getIntegerAttr(value.getType(), *maybeConstValue);
- Value constant =
- folder.getOrCreateConstant(b.getInsertionBlock(), valueDialect, constAttr,
- value.getType(), value.getLoc());
+ Value constant = folder.getOrCreateConstant(
+ b.getInsertionBlock(), valueDialect, constAttr, value.getType());
if (!constant)
return failure();