[Mlir-commits] [mlir] [mlir][toy] Update dialect conversion example (PR #150826)

[Matthias Springer]

================
@@ -223,8 +221,11 @@ void ToyToLLVMLoweringPass::runOnOperation() {
   populateFuncToLLVMConversionPatterns(typeConverter, patterns);
 
   // The only remaining operation to lower from the `toy` dialect, is the
-  // PrintOp.
-  patterns.add<PrintOpLowering>(&getContext());
+  // PrintOp. An identity converter is needed because the PrintOp lowering
+  // operates on MemRefType instead of the lowered LLVM struct type.
+  TypeConverter identityConverter;
+  identityConverter.addConversion([](Type type) { return type; });
+  patterns.add<PrintOpLowering>(identityConverter, &getContext());
----------------
matthias-springer wrote:

> using the operand to construct new IR

I meant: inside of a `ConversionPattern`, retrieve an operand from the matched op, and use that SSA value to construct a new op.

Inside of a `ConversionPattern`, the main difference between picking (a) a value from the adaptor or (b) the respective operand from the matched op is the type of the SSA value that you get. (b) always has the original type. (a) may be a type-converted "version" of (b).

I was worried that (b) is not safe because the defining op of that value may have been scheduled for erasure, leaving us with an operand pointing to an erased op. But I think that cannot happen. When an op is replaced (or erased), its results are mapped to something else in the `ConversionValueMapping`. During the "commit" phase, those mapping are materialized in IR via `replaceAllUsesWith`. At that point, the operand of the newly-constructed op in the example above will be switched to an SSA value that survives the conversion and has the correct type. (A materialization may be created at that point of time.)



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