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

[llvmlistbot at llvm.org]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-mlir

Author: Maksim Levental (makslevental)

<details>
<summary>Changes</summary>

`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.


---
Full diff: https://github.com/llvm/llvm-project/pull/123154.diff


1 Files Affected:

- (modified) mlir/lib/Transforms/Utils/DialectConversion.cpp (+1-1) 


``````````diff
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;

``````````

</details>


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