Skip to content

Commit 2a9d8ca

Browse files
committed
Revert "[MLIR] Fuse locations of merged constants (#74670)"
This reverts commit 87e2e89. and its follow-ups 0d1490f (#75218) and 6fe3cd5 (#75312). We observed significant OOM/timeout issues due to #74670 to quite a few services including google-research/swirl-lm. The follow-up #75218 and #75312 do not address the issue. Perhaps this is worth more investigation.
1 parent c64334f commit 2a9d8ca

File tree

4 files changed

+2
-164
lines changed

4 files changed

+2
-164
lines changed

mlir/include/mlir/Transforms/FoldUtils.h

Lines changed: 1 addition & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -33,8 +33,7 @@ class Value;
3333
class OperationFolder {
3434
public:
3535
OperationFolder(MLIRContext *ctx, OpBuilder::Listener *listener = nullptr)
36-
: fusedLocationTag(StringAttr::get(ctx, "CSE")), interfaces(ctx),
37-
rewriter(ctx, listener) {}
36+
: interfaces(ctx), rewriter(ctx, listener) {}
3837

3938
/// Tries to perform folding on the given `op`, including unifying
4039
/// deduplicated constants. If successful, replaces `op`'s uses with
@@ -96,15 +95,6 @@ class OperationFolder {
9695
Dialect *dialect, Attribute value,
9796
Type type, Location loc);
9897

99-
// Fuse `foldedLocation` into the Location of `retainedOp`. This will result
100-
// in `retainedOp` having a FusedLoc with `fusedLocationTag` to help trace the
101-
// source of the fusion. If `retainedOp` already had a FusedLoc with the same
102-
// tag, `foldedLocation` will simply be appended to it.
103-
void appendFoldedLocation(Operation *retainedOp, Location foldedLocation);
104-
105-
/// Tag for annotating fused locations as a result of merging constants.
106-
StringAttr fusedLocationTag;
107-
10898
/// A mapping between an insertion region and the constants that have been
10999
/// created within it.
110100
DenseMap<Region *, ConstantMap> foldScopes;

mlir/lib/Transforms/Utils/FoldUtils.cpp

Lines changed: 1 addition & 78 deletions
Original file line numberDiff line numberDiff line change
@@ -141,7 +141,6 @@ bool OperationFolder::insertKnownConstant(Operation *op, Attribute constValue) {
141141
// If there is an existing constant, replace `op`.
142142
if (folderConstOp) {
143143
notifyRemoval(op);
144-
appendFoldedLocation(folderConstOp, op->getLoc());
145144
rewriter.replaceOp(op, folderConstOp->getResults());
146145
return false;
147146
}
@@ -295,10 +294,8 @@ OperationFolder::tryGetOrCreateConstant(ConstantMap &uniquedConstants,
295294
// Check if an existing mapping already exists.
296295
auto constKey = std::make_tuple(dialect, value, type);
297296
Operation *&constOp = uniquedConstants[constKey];
298-
if (constOp) {
299-
appendFoldedLocation(constOp, loc);
297+
if (constOp)
300298
return constOp;
301-
}
302299

303300
// If one doesn't exist, try to materialize one.
304301
if (!(constOp = materializeConstant(dialect, rewriter, value, type, loc)))
@@ -319,7 +316,6 @@ OperationFolder::tryGetOrCreateConstant(ConstantMap &uniquedConstants,
319316
// materialized operation in favor of the existing one.
320317
if (auto *existingOp = uniquedConstants.lookup(newKey)) {
321318
notifyRemoval(constOp);
322-
appendFoldedLocation(existingOp, constOp->getLoc());
323319
rewriter.eraseOp(constOp);
324320
referencedDialects[existingOp].push_back(dialect);
325321
return constOp = existingOp;
@@ -330,76 +326,3 @@ OperationFolder::tryGetOrCreateConstant(ConstantMap &uniquedConstants,
330326
auto newIt = uniquedConstants.insert({newKey, constOp});
331327
return newIt.first->second;
332328
}
333-
334-
/// Helper that flattens nested fused locations to a single fused location.
335-
/// Fused locations nested under non-fused locations are not flattened, and
336-
/// calling this on non-fused locations is a no-op as a result.
337-
///
338-
/// Fused locations are only flattened into parent fused locations if the
339-
/// child fused location has no metadata, or if the metadata of the parent and
340-
/// child fused locations are the same---this to avoid breaking cases where
341-
/// metadata matter.
342-
static Location FlattenFusedLocationRecursively(const Location loc) {
343-
auto fusedLoc = dyn_cast<FusedLoc>(loc);
344-
if (!fusedLoc)
345-
return loc;
346-
347-
SetVector<Location> flattenedLocs;
348-
Attribute metadata = fusedLoc.getMetadata();
349-
ArrayRef<Location> unflattenedLocs = fusedLoc.getLocations();
350-
bool hasAnyNestedLocChanged = false;
351-
352-
for (const Location &unflattenedLoc : unflattenedLocs) {
353-
Location flattenedLoc = FlattenFusedLocationRecursively(unflattenedLoc);
354-
355-
auto flattenedFusedLoc = dyn_cast<FusedLoc>(flattenedLoc);
356-
if (flattenedFusedLoc && (!flattenedFusedLoc.getMetadata() ||
357-
flattenedFusedLoc.getMetadata() == metadata)) {
358-
hasAnyNestedLocChanged = true;
359-
ArrayRef<Location> nestedLocations = flattenedFusedLoc.getLocations();
360-
flattenedLocs.insert(nestedLocations.begin(), nestedLocations.end());
361-
} else {
362-
if (flattenedLoc != unflattenedLoc)
363-
hasAnyNestedLocChanged = true;
364-
365-
flattenedLocs.insert(flattenedLoc);
366-
}
367-
}
368-
369-
if (!hasAnyNestedLocChanged &&
370-
unflattenedLocs.size() == flattenedLocs.size()) {
371-
return loc;
372-
}
373-
374-
return FusedLoc::get(loc->getContext(), flattenedLocs.takeVector(),
375-
fusedLoc.getMetadata());
376-
}
377-
378-
void OperationFolder::appendFoldedLocation(Operation *retainedOp,
379-
Location foldedLocation) {
380-
// Append into existing fused location if it has the same tag.
381-
if (auto existingFusedLoc =
382-
dyn_cast<FusedLocWith<StringAttr>>(retainedOp->getLoc())) {
383-
StringAttr existingMetadata = existingFusedLoc.getMetadata();
384-
if (existingMetadata == fusedLocationTag) {
385-
ArrayRef<Location> existingLocations = existingFusedLoc.getLocations();
386-
SetVector<Location> locations(existingLocations.begin(),
387-
existingLocations.end());
388-
locations.insert(foldedLocation);
389-
Location newFusedLoc = FusedLoc::get(
390-
retainedOp->getContext(), locations.takeVector(), existingMetadata);
391-
retainedOp->setLoc(FlattenFusedLocationRecursively(newFusedLoc));
392-
return;
393-
}
394-
}
395-
396-
// Create a new fusedloc with retainedOp's loc and foldedLocation.
397-
// If they're already equal, no need to fuse.
398-
if (retainedOp->getLoc() == foldedLocation)
399-
return;
400-
401-
Location newFusedLoc =
402-
FusedLoc::get(retainedOp->getContext(),
403-
{retainedOp->getLoc(), foldedLocation}, fusedLocationTag);
404-
retainedOp->setLoc(FlattenFusedLocationRecursively(newFusedLoc));
405-
}

mlir/test/Transforms/canonicalize-debuginfo.mlir

Lines changed: 0 additions & 41 deletions
This file was deleted.

mlir/test/Transforms/constant-fold-debuginfo.mlir

Lines changed: 0 additions & 34 deletions
This file was deleted.

0 commit comments

Comments
 (0)