[Mlir-commits] [mlir] [mlir][Transforms] Dialect conversion: Fix missing source materialization (PR #97903)

Matthias Springer llvmlistbot at llvm.org
Sun Jul 7 09:34:21 PDT 2024


================
@@ -2844,18 +2847,10 @@ static LogicalResult legalizeUnresolvedMaterialization(
     switch (mat.getMaterializationKind()) {
     case MaterializationKind::Argument:
----------------
matthias-springer wrote:

Honestly, that comment makes no sense to me. This part of the code base deals exclusively with materializations; there are no type conversions anymore.

When this comment was added, the implementation already handled both argument and target materializations. Source materializations are not handled here because they never show up as "unresolved materializations". There isn't even a `MaterializationKind::Source` enum value. So I think this should be rephrased as `We currently handle only argument and target materializations here.` I believe if we were to handle source materializations here, a few `unrealized_conversion_cast` ops (the ones that cancel out with target materializations) would not have to be materialized. So the comment could be a kind of TODO to support source materializations.

```
// We currently only handle target materializations here.
OpResult opResult = op->getOpResult(0);
```

Interestingly this comment is right before the `getOpResult(0)`. Another limitation of this part of the code base is that 1:N materializations are not supported. (But neither does the type converter API support it when adding materialization functions.)


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


More information about the Mlir-commits mailing list