Author: Alexander Belyaev Date: 2020-12-16T23:09:04+01:00 New Revision: 9ca67d7f4467a5dab4287e7c431f9daf313ca38a
URL: https://github.com/llvm/llvm-project/commit/9ca67d7f4467a5dab4287e7c431f9daf313ca38a DIFF: https://github.com/llvm/llvm-project/commit/9ca67d7f4467a5dab4287e7c431f9daf313ca38a.diff LOG: Revert "[mlir] Lookup the latest value with a legal type when remapping values." This reverts commit f8184d4c44dff1fab13122221f0c23ab50936647. Added: Modified: mlir/lib/Transforms/Utils/DialectConversion.cpp Removed: ################################################################################ diff --git a/mlir/lib/Transforms/Utils/DialectConversion.cpp b/mlir/lib/Transforms/Utils/DialectConversion.cpp index 61371b2c08b6..0a1a6b712ff2 100644 --- a/mlir/lib/Transforms/Utils/DialectConversion.cpp +++ b/mlir/lib/Transforms/Utils/DialectConversion.cpp @@ -105,15 +105,10 @@ namespace { /// functionality, i.e. we will traverse if the mapped value also has a mapping. struct ConversionValueMapping { /// Lookup a mapped value within the map. If a mapping for the provided value - /// does not exist then return the provided value. - Value lookupOrDefault(Value from) const; - - /// Lookup the latest legal value within the map. If a mapping for the - /// provided value does not exist then return the provided value. If - /// `converter` is non-null, returns the most recently mapped value with the - /// legal type. If an operand of that type does not exist, defaults to normal - /// behavior. - Value lookupLatestLegal(Value from, TypeConverter *converter) const; + /// does not exist then return the provided value. If `desiredType` is + /// non-null, returns the most recently mapped value with that type. If an + /// operand of that type does not exist, defaults to normal behavior. + Value lookupOrDefault(Value from, Type desiredType = nullptr) const; /// Lookup a mapped value within the map, or return null if a mapping does not /// exist. If a mapping exists, this follows the same behavior of @@ -132,24 +127,22 @@ struct ConversionValueMapping { }; } // end anonymous namespace -Value ConversionValueMapping::lookupOrDefault(Value from) const { - // If this value had a valid mapping, unmap that value as well in the case - // that it was also replaced. - while (auto mappedValue = mapping.lookupOrNull(from)) - from = mappedValue; - return from; -} - -Value ConversionValueMapping::lookupLatestLegal( - Value from, TypeConverter *converter) const { - if (!converter) - return lookupOrDefault(from); +Value ConversionValueMapping::lookupOrDefault(Value from, + Type desiredType) const { + // If there was no desired type, simply find the leaf value. + if (!desiredType) { + // If this value had a valid mapping, unmap that value as well in the case + // that it was also replaced. + while (auto mappedValue = mapping.lookupOrNull(from)) + from = mappedValue; + return from; + } - // Otherwise, try to find the deepest value that has the legal type. - Value legalValue; + // Otherwise, try to find the deepest value that has the desired type. + Value desiredValue; do { - if (converter->isLegal(from.getType())) - legalValue = from; + if (from.getType() == desiredType) + desiredValue = from; Value mappedValue = mapping.lookupOrNull(from); if (!mappedValue) @@ -158,7 +151,7 @@ Value ConversionValueMapping::lookupLatestLegal( } while (true); // If the desired value was found use it, otherwise default to the leaf value. - return legalValue ? legalValue : from; + return desiredValue ? desiredValue : from; } Value ConversionValueMapping::lookupOrNull(Value from) const { @@ -1046,41 +1039,22 @@ LogicalResult ConversionPatternRewriterImpl::remapValues( Value operand = it.value(); Type origType = operand.getType(); - Value newOperand = mapping.lookupLatestLegal(operand, converter); - - // Handle the case where the conversion was 1->1 and the new operand type - // isn't legal. - Type newOperandType = newOperand.getType(); + // If a converter was provided, get the desired legal types for this + // operand. + Type desiredType; if (converter) { - if (!converter->isLegal(newOperandType)) { - legalTypes.clear(); - - // If there is no legal conversion, fail to match this pattern. - if (failed(converter->convertType(origType, legalTypes))) { - return notifyMatchFailure(loc, [=](Diagnostic &diag) { - diag << "unable to convert type for operand #" << it.index() - << ", type was " << origType; - }); - } - // TODO: There currently isn't any mechanism to do 1->N type conversion - // via the PatternRewriter replacement API, so for now we just ignore - // it. - if (legalTypes.size() != 1) { - remapped.push_back(newOperand); - continue; - } - Type desiredType = legalTypes.front(); - newOperand = converter->materializeTargetConversion( - rewriter, loc, desiredType, newOperand); - if (!newOperand) { - return notifyMatchFailure(loc, [=](Diagnostic &diag) { - diag << "unable to materialize a conversion for " - "operand #" - << it.index() << ", from " << newOperandType << " to " - << desiredType; - }); - } + // If there is no legal conversion, fail to match this pattern. + legalTypes.clear(); + if (failed(converter->convertType(origType, legalTypes))) { + return notifyMatchFailure(loc, [=](Diagnostic &diag) { + diag << "unable to convert type for operand #" << it.index() + << ", type was " << origType; + }); } + // TODO: There currently isn't any mechanism to do 1->N type conversion + // via the PatternRewriter replacement API, so for now we just ignore it. + if (legalTypes.size() == 1) + desiredType = legalTypes.front(); } else { // TODO: What we should do here is just set `desiredType` to `origType` // and then handle the necessary type conversions after the conversion @@ -1088,7 +1062,24 @@ LogicalResult ConversionPatternRewriterImpl::remapValues( // receiving the new operands even if the types change, so we keep the // original behavior here for now until all of the patterns relying on // this get updated. + } + Value newOperand = mapping.lookupOrDefault(operand, desiredType); + + // Handle the case where the conversion was 1->1 and the new operand type + // isn't legal. + Type newOperandType = newOperand.getType(); + if (converter && desiredType && newOperandType != desiredType) { // Attempt to materialize a conversion for this new value. + newOperand = converter->materializeTargetConversion( + rewriter, loc, desiredType, newOperand); + if (!newOperand) { + return notifyMatchFailure(loc, [=](Diagnostic &diag) { + diag << "unable to materialize a conversion for " + "operand #" + << it.index() << ", from " << newOperandType << " to " + << desiredType; + }); + } } remapped.push_back(newOperand); } _______________________________________________ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
