[Mlir-commits] [mlir] [mlir][LLVM] Add the `ConvertToLLVMAttrInterface` and `ConvertToLLVMOpInterface` interfaces (PR #99566)

[Fabian Mora]

================
@@ -104,20 +99,33 @@ class ConvertToLLVMPass
         auto *iface = dyn_cast<ConvertToLLVMPatternInterface>(dialect);
         if (!iface)
           continue;
-        iface->populateConvertToLLVMConversionPatterns(*target, *typeConverter,
-                                                       tempPatterns);
+        interfaces->push_back(iface);
       }
     }
 
-    this->patterns =
-        std::make_unique<FrozenRewritePatternSet>(std::move(tempPatterns));
-    this->target = target;
-    this->typeConverter = typeConverter;
+    this->interfaces = interfaces;
     return success();
   }
 
   void runOnOperation() final {
-    if (failed(applyPartialConversion(getOperation(), *target, *patterns)))
+    MLIRContext *context = &getContext();
+    RewritePatternSet patterns(context);
+    ConversionTarget target(*context);
+    target.addLegalDialect<LLVM::LLVMDialect>();
+    LLVMTypeConverter typeConverter(context);
----------------
fabianmcg wrote:

I can profile it and check the impact.

However, let's say the cost of that initialization is `C`. In the previous version `C` is paid only once.
In this version the overall cost is `C` * `threads`, so the overall computation time does increment.
However, on a thread per thread basis the cost remains relatively the same.

So, I would argue, that unless the effect of the change is severe, the extra overhead might be justified for the feature.

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