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

Mon Jul 29 12:23:57 PDT 2024

fabianmcg wrote:

@joker-eph I added an option to enable or disable using conversion attributes, making the performance cost optional.
I also updated `populateOpConvertToLLVMConversionPatterns` to check the op or parent ops for the conversion op interface.

I also conducted several other tests.
## First test
The first test was about the overhead of the change.
The commands used for testing were:
```
// Use the slow path
mlir-opt --pass-pipeline="builtin.module(convert-to-llvm{use-conversion-attrs=true})"  mlir/test/Conversion/FuncToLLVM/func-to-llvm.mlir --mlir-timing
// Use the fast path
mlir-opt --pass-pipeline="builtin.module(convert-to-llvm{use-conversion-attrs=false})"  mlir/test/Conversion/FuncToLLVM/func-to-llvm.mlir --mlir-timing
// Upstream
mlir-opt --pass-pipeline="builtin.module(convert-to-llvm)"  mlir/test/Conversion/FuncToLLVM/func-to-llvm.mlir --mlir-timing
```

Each of the commands were run 1000 times, under the following settings:
```
Build: LLVM Release with assertions off
CPU: Intel(R) Xeon(R) Gold 6132 CPU @ 2.60GHz
```

The results were:
<html xmlns:v="urn:schemas-microsoft-com:vml"
xmlns:o="urn:schemas-microsoft-com:office:office"
xmlns:x="urn:schemas-microsoft-com:office:excel"
xmlns="http://www.w3.org/TR/REC-html40">

<body link="#467886" vlink="#96607D">

  | C1: use-conversion-attrs=true | C2: use-conversion-attrs=false | C3: upstream | C1-C2 | C2-C3 | C1-C3
-- | -- | -- | -- | -- | -- | --
Pass   Manager: (s) | 0.002824224 | 0.002724727 | 0.002791926 | 9.94975E-05 | -6.71990E-05 | 3.22985E-05
ConvertToLLVM:   (s) | 0.002065047 | 0.001913615 | 0.001976492 | 1.51432E-04 | -6.28765E-05 | 8.85554E-05
Total:   (s) | 0.00545438 | 0.005311841 | 0.005429315 | 1.42539E-04 | -1.17474E-04 | 2.50649E-05
</body>
</html>

The pass manager times correspond to the times obtained by modifying `MlirOptMain` with:
```c++
  auto start = std::chrono::high_resolution_clock::now();
  if (failed(pm.run(*op)))
    return failure();
  auto stop = std::chrono::high_resolution_clock::now();
  auto duration = std::chrono::duration_cast<std::chrono::duration<double>>(stop -start);
```

These results show that the difference in performance between upstream and the change with `use-conversion-attrs=false` is nonexistent.  They also show that under the current testing conditions the performance incurred by `use-conversion-attrs=true` is almost nonexistent.

## Second test
I added the following lines to `ConvertToLLVMPass::initialize`:

```c++
  static int x = 0;
  llvm::errs() << "init: " << x++ << "\n";
```

Then run the pipeline `builtin.module(convert-to-llvm, convert-to-llvm)`, using the following C++ code:
```c++
  if (failed(pm.run(*op)))
    return failure();
  if (failed(pm.run(*op)))
    return failure();
```

The output was:
```
init: 0
init: 1
init: 2
init: 3
```

This means that the `PassManager` is not caching `Pass::initialize` . Hence, this change introduces no overhead under the current conditions (even with ` use-conversion-attrs=true`).

One final thing I discovered is that applying `builtin.module(func.func(convert-to-llvm))` segfaults because the pass attempts to convert `func.func` . Hence, the pass can only applied at the "module" level. 

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