[mlir] Use a container with deterministic iteration order for unrealized materializations (#191323)
Iteration over unrealized materializations in DialectConversion is non-deterministic as the materialization metadata is stored in a DenseMap. Replacing with a Vector-backed `llvm::MapVector` restores deterministic behaviour. This container is iterated for example here: https://github.com/llvm/llvm-project/blob/main/mlir/lib/Transforms/Utils/DialectConversion.cpp#L3250
This commit is contained in:
@@ -1139,7 +1139,7 @@ struct ConversionPatternRewriterImpl : public RewriterBase::Listener {
|
||||
DenseSet<UnrealizedConversionCastOp> patternMaterializations;
|
||||
|
||||
/// A mapping for looking up metadata of unresolved materializations.
|
||||
DenseMap<UnrealizedConversionCastOp, UnresolvedMaterializationInfo>
|
||||
llvm::MapVector<UnrealizedConversionCastOp, UnresolvedMaterializationInfo>
|
||||
unresolvedMaterializations;
|
||||
|
||||
/// The current type converter, or nullptr if no type converter is currently
|
||||
@@ -3243,8 +3243,8 @@ void mlir::reconcileUnrealizedCasts(
|
||||
|
||||
namespace mlir {
|
||||
static void reconcileUnrealizedCasts(
|
||||
const DenseMap<UnrealizedConversionCastOp, UnresolvedMaterializationInfo>
|
||||
&castOps,
|
||||
const llvm::MapVector<UnrealizedConversionCastOp,
|
||||
UnresolvedMaterializationInfo> &castOps,
|
||||
SmallVectorImpl<UnrealizedConversionCastOp> *remainingCastOps) {
|
||||
reconcileUnrealizedCastsImpl(
|
||||
castOps.keys(),
|
||||
@@ -3486,8 +3486,9 @@ LogicalResult OperationConverter::applyConversion(ArrayRef<Operation *> ops) {
|
||||
// Reconcile all UnrealizedConversionCastOps that were inserted by the
|
||||
// dialect conversion frameworks. (Not the ones that were inserted by
|
||||
// patterns.)
|
||||
const DenseMap<UnrealizedConversionCastOp, UnresolvedMaterializationInfo>
|
||||
&materializations = rewriterImpl.unresolvedMaterializations;
|
||||
const llvm::MapVector<UnrealizedConversionCastOp,
|
||||
UnresolvedMaterializationInfo> &materializations =
|
||||
rewriterImpl.unresolvedMaterializations;
|
||||
SmallVector<UnrealizedConversionCastOp> remainingCastOps;
|
||||
reconcileUnrealizedCasts(materializations, &remainingCastOps);
|
||||
|
||||
|
||||
@@ -184,3 +184,19 @@ gpu.module @cuda_events {
|
||||
gpu.return
|
||||
}
|
||||
}
|
||||
|
||||
// -----
|
||||
|
||||
// Test that the order of materialization legalization is deterministic.
|
||||
// Multiple unresolvable materializations are created; the first one (in
|
||||
// insertion/walk order) should always fail first. With a DenseMap for
|
||||
// unresolvedMaterializations, the processing order would depend on pointer
|
||||
// hashes and could differ with LLVM_ENABLE_REVERSE_ITERATION.
|
||||
|
||||
func.func @test_deterministic_materialization_order() {
|
||||
// expected-error@below {{failed to legalize unresolved materialization from ('f64') to ('f16') that remained live after conversion}}
|
||||
%a = "test.type_producer"() : () -> f16
|
||||
%b = "test.type_producer"() : () -> f16
|
||||
// expected-note@below {{see existing live user here}}
|
||||
"foo.return"(%a, %b) : (f16, f16) -> ()
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user