address comments

Author: matthias-springer

mlir/docs/Tutorials/Toy/Ch-6.md

@@ -81,14 +81,6 @@ enough for our use case.
   LLVMTypeConverter typeConverter(&getContext());
 ```
 
-For the `toy.print` lowering, we need a special type converter to ensure that
-the pattern receives a `memref` value in its adaptor. If we were to use the
-LLVM type converter, it would receive an `llvm.struct`, which is the normal
-lowering of a `memref` type to LLVM. If we were to use no type converter at
-all, it would receive a value with the original tensor type. (Note: The dialect
-conversion driver currently passes the "most recently mapped value", i.e., a
-value of unspecified type. This is a bug in the conversion driver.)
-
 ### Conversion Patterns
 
 Now that the conversion target has been defined, we need to provide the patterns

mlir/examples/toy/Ch6/mlir/LowerToLLVM.cpp

@@ -108,7 +108,7 @@ class PrintOpLowering : public OpConversionPattern<toy::PrintOp> {
 
     // Generate a call to printf for the current element of the loop.
     auto elementLoad =
-        memref::LoadOp::create(rewriter, loc, adaptor.getInput(), loopIvs);
+        memref::LoadOp::create(rewriter, loc, op.getInput(), loopIvs);
     LLVM::CallOp::create(rewriter, loc, getPrintfType(context), printfRef,
                          ArrayRef<Value>({formatSpecifierCst, elementLoad}));
 
@@ -221,11 +221,8 @@ void ToyToLLVMLoweringPass::runOnOperation() {
   populateFuncToLLVMConversionPatterns(typeConverter, patterns);
 
   // The only remaining operation to lower from the `toy` dialect, is the
-  // 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());
+  // PrintOp.
+  patterns.add<PrintOpLowering>(&getContext());
 
   // We want to completely lower to LLVM, so we use a `FullConversion`. This
   // ensures that only legal operations will remain after the conversion.

mlir/examples/toy/Ch7/mlir/LowerToLLVM.cpp

@@ -108,7 +108,7 @@ class PrintOpLowering : public OpConversionPattern<toy::PrintOp> {
 
     // Generate a call to printf for the current element of the loop.
     auto elementLoad =
-        memref::LoadOp::create(rewriter, loc, adaptor.getInput(), loopIvs);
+        memref::LoadOp::create(rewriter, loc, op.getInput(), loopIvs);
     LLVM::CallOp::create(rewriter, loc, getPrintfType(context), printfRef,
                          ArrayRef<Value>({formatSpecifierCst, elementLoad}));
 
@@ -222,11 +222,8 @@ void ToyToLLVMLoweringPass::runOnOperation() {
   populateFuncToLLVMConversionPatterns(typeConverter, patterns);
 
   // The only remaining operation to lower from the `toy` dialect, is the
-  // 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());
+  // PrintOp.
+  patterns.add<PrintOpLowering>(&getContext());
 
   // We want to completely lower to LLVM, so we use a `FullConversion`. This
   // ensures that only legal operations will remain after the conversion.