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
_______________________________________________
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

Reply via email to