Skip to content

Commit 486f83f

Browse files
[mlir][Transforms][NFC] Simplify buildUnresolvedMaterialization implementation (#121651)
The `buildUnresolvedMaterialization` implementation used to check if a materialization is necessary. A materialization is not necessary if the desired types already match the input. However, this situation can never happen: we look for mapped values with the desired type at the call sites before requesting a new unresolved materialization. The previous implementation seemed incorrect because `buildUnresolvedMaterialization` created a mapping that is never rolled back. (When in reality that code was never executed, so it is technically not incorrect.) Also fix a comment that in `findOrBuildReplacementValue` that was incorrect.
1 parent 8b57704 commit 486f83f

File tree

1 file changed

+5
-12
lines changed

1 file changed

+5
-12
lines changed

mlir/lib/Transforms/Utils/DialectConversion.cpp

Lines changed: 5 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1430,13 +1430,8 @@ ValueRange ConversionPatternRewriterImpl::buildUnresolvedMaterialization(
14301430
UnrealizedConversionCastOp *castOp) {
14311431
assert((!originalType || kind == MaterializationKind::Target) &&
14321432
"original type is valid only for target materializations");
1433-
1434-
// Avoid materializing an unnecessary cast.
1435-
if (TypeRange(inputs) == outputTypes) {
1436-
if (!valuesToMap.empty())
1437-
mapping.map(std::move(valuesToMap), inputs);
1438-
return inputs;
1439-
}
1433+
assert(TypeRange(inputs) != outputTypes &&
1434+
"materialization is not necessary");
14401435

14411436
// Create an unresolved materialization. We use a new OpBuilder to avoid
14421437
// tracking the materialization like we do for other operations.
@@ -1455,7 +1450,9 @@ ValueRange ConversionPatternRewriterImpl::buildUnresolvedMaterialization(
14551450

14561451
Value ConversionPatternRewriterImpl::findOrBuildReplacementValue(
14571452
Value value, const TypeConverter *converter) {
1458-
// Find a replacement value with the same type.
1453+
// Try to find a replacement value with the same type in the conversion value
1454+
// mapping. This includes cached materializations. We try to reuse those
1455+
// instead of generating duplicate IR.
14591456
ValueVector repl = mapping.lookupOrNull(value, value.getType());
14601457
if (!repl.empty())
14611458
return repl.front();
@@ -1489,10 +1486,6 @@ Value ConversionPatternRewriterImpl::findOrBuildReplacementValue(
14891486
// in the conversion value mapping.) The insertion point of the
14901487
// materialization must be valid for all future users that may be created
14911488
// later in the conversion process.
1492-
//
1493-
// Note: Instead of creating new IR, `buildUnresolvedMaterialization` may
1494-
// return an already existing, cached materialization from the conversion
1495-
// value mapping.
14961489
Value castValue =
14971490
buildUnresolvedMaterialization(MaterializationKind::Source,
14981491
computeInsertPoint(repl), value.getLoc(),

0 commit comments

Comments
 (0)