[Mlir-commits] [mlir] [mlir] fix dialect conversion with no type converter (PR #123154)

[Maksim Levental]

https://github.com/makslevental created https://github.com/llvm/llvm-project/pull/123154

`ConversionPatternRewriterImpl::remapValues` has a bug when no type converter is used for a 1:N dialect conversion:

```c++
if (!currentTypeConverter) {
  // The current pattern does not have a type converter. I.e., it does not
  // distinguish between legal and illegal types. For each operand, simply
  // pass through the most recently mapped values.
  remapped.push_back(mapping.lookupOrDefault(operand));
  continue;
}
```

Emphasis on `most recently mapped values`. But the implementation of `mapping.lookupOrDefault` in such a case always recurse all the way back to the leaf values:

```c++
ValueVector
ConversionValueMapping::lookupOrDefault(...) const {
  ...
  do {
    ...
    if (TypeRange(ValueRange(current)) == desiredTypes)
      desiredValue = current;
    ....
    if (next != current) {
      // If at least one value was replaced, continue the lookup from there.
      current = std::move(next);
      continue;
    }
    ...
    if (it == mapping.end()) {
      // No mapping found: The lookup stops here.
      break;
    }
    current = it->second;
  } while (true);

  // If the desired values were found use them, otherwise default to the leaf values.
  return !desiredValue.empty() ? std::move(desiredValue) : std::move(current);
}
```

Note, that `desiredTypes` is empty and thus `desiredValue.empty()` is always true and thus `std::move(current)` is returned i.e., the leaf nodes. 

The fix is to condition continuing the while loop on `!desiredTypes.empty()` and otherwise break with `current` containing the first mapping lookup.


>From e548a3e3a04cef5c43320ed0a799116d59812c17 Mon Sep 17 00:00:00 2001
From: max <maksim.levental at gmail.com>
Date: Wed, 15 Jan 2025 23:16:32 -0500
Subject: [PATCH] [mlir] fix dialect conversion with no type converter

---
 mlir/lib/Transforms/Utils/DialectConversion.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mlir/lib/Transforms/Utils/DialectConversion.cpp b/mlir/lib/Transforms/Utils/DialectConversion.cpp
index 403321d40d53c9..34e5d56bd95055 100644
--- a/mlir/lib/Transforms/Utils/DialectConversion.cpp
+++ b/mlir/lib/Transforms/Utils/DialectConversion.cpp
@@ -208,7 +208,7 @@ ConversionValueMapping::lookupOrDefault(Value from,
         next.push_back(v);
       }
     }
-    if (next != current) {
+    if (!desiredTypes.empty() && next != current) {
       // If at least one value was replaced, continue the lookup from there.
       current = std::move(next);
       continue;