Skip to content

Commit fde344a

Browse files
[mlir][Transforms] Dialect conversion: Improve signature conversion API (#81997)
This commit improves the block signature conversion API of the dialect conversion. There is the following comment in `ArgConverter::applySignatureConversion`: ``` // If no arguments are being changed or added, there is nothing to do. ``` However, the implementation actually used to replace a block with a new block even if the block argument types do not change (i.e., there is "nothing to do"). This is fixed in this commit. The documentation of the public `ConversionPatternRewriter` API is updated accordingly. This commit also removes a check that used to *sometimes* skip a block signature conversion if the block was already converted. This is not consistent with the public `ConversionPatternRewriter` API; blocks should always be converted, regardless of whether they were already converted or not. Block signature conversion also used to be silently skipped when the specified block was detached. Instead of silently skipping, an assertion is triggered. Attempting to convert a detached block (which is likely an erased block) is invalid API usage.
1 parent 8bd327d commit fde344a

File tree

2 files changed

+12
-10
lines changed

2 files changed

+12
-10
lines changed

mlir/include/mlir/Transforms/DialectConversion.h

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -663,6 +663,8 @@ class ConversionPatternRewriter final : public PatternRewriter {
663663
/// Apply a signature conversion to the entry block of the given region. This
664664
/// replaces the entry block with a new block containing the updated
665665
/// signature. The new entry block to the region is returned for convenience.
666+
/// If no block argument types are changing, the entry original block will be
667+
/// left in place and returned.
666668
///
667669
/// If provided, `converter` will be used for any materializations.
668670
Block *
@@ -671,8 +673,11 @@ class ConversionPatternRewriter final : public PatternRewriter {
671673
const TypeConverter *converter = nullptr);
672674

673675
/// Convert the types of block arguments within the given region. This
674-
/// replaces each block with a new block containing the updated signature. The
675-
/// entry block may have a special conversion if `entryConversion` is
676+
/// replaces each block with a new block containing the updated signature. If
677+
/// an updated signature would match the current signature, the respective
678+
/// block is left in place as is.
679+
///
680+
/// The entry block may have a special conversion if `entryConversion` is
676681
/// provided. On success, the new entry block to the region is returned for
677682
/// convenience. Otherwise, failure is returned.
678683
FailureOr<Block *> convertRegionTypes(
@@ -681,7 +686,8 @@ class ConversionPatternRewriter final : public PatternRewriter {
681686

682687
/// Convert the types of block arguments within the given region except for
683688
/// the entry region. This replaces each non-entry block with a new block
684-
/// containing the updated signature.
689+
/// containing the updated signature. If an updated signature would match the
690+
/// current signature, the respective block is left in place as is.
685691
///
686692
/// If special conversion behavior is needed for the non-entry blocks (for
687693
/// example, we need to convert only a subset of a BB arguments), such

mlir/lib/Transforms/Utils/DialectConversion.cpp

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -544,12 +544,8 @@ FailureOr<Block *> ArgConverter::convertSignature(
544544
Block *block, const TypeConverter *converter,
545545
ConversionValueMapping &mapping,
546546
SmallVectorImpl<BlockArgument> &argReplacements) {
547-
// Check if the block was already converted.
548-
// * If the block is mapped in `conversionInfo`, it is a converted block.
549-
// * If the block is detached, conservatively assume that it is going to be
550-
// deleted; it is likely the old block (before it was converted).
551-
if (conversionInfo.count(block) || !block->getParent())
552-
return block;
547+
assert(block->getParent() && "cannot convert signature of detached block");
548+
553549
// If a converter wasn't provided, and the block wasn't already converted,
554550
// there is nothing we can do.
555551
if (!converter)
@@ -570,7 +566,7 @@ Block *ArgConverter::applySignatureConversion(
570566
// If no arguments are being changed or added, there is nothing to do.
571567
unsigned origArgCount = block->getNumArguments();
572568
auto convertedTypes = signatureConversion.getConvertedTypes();
573-
if (origArgCount == 0 && convertedTypes.empty())
569+
if (llvm::equal(block->getArgumentTypes(), convertedTypes))
574570
return block;
575571

576572
// Split the block at the beginning to get a new block to use for the updated

0 commit comments

Comments
 (0)