[llvm-branch-commits] [mlir] [mlir][Transforms] Dialect Conversion: Add `replaceOpWithMultiple` (PR #115816)

Markus Böck via llvm-branch-commits llvm-branch-commits at lists.llvm.org
Tue Nov 12 02:46:11 PST 2024


https://github.com/zero9178 commented:

Looks mostly good to me besides some code nits :slightly_smiling_face: 

The only very high level conern I have is the use and need of `InsertionPoint::after`. As I understand, this is a not-yet-implemented optimization for the future for the purpose of reusing materializations by calculating a better insertion point where it may dominate more operations which might need exactly that materialization after pattern application as well. 

Is the current complexity of calculating the insertion point (and potentially calculatin the post-dom tree, a `O(n log n)` operation) worth the gain of the optimization? I am thinking that the better insertion point insertion logic should probably be post-poned until we can measure its effectivness (and avoid the risk of a premature optimization) and have something simpler and working that does not worsen the status-quo for now instead.

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


More information about the llvm-branch-commits mailing list