[Mlir-commits] [mlir] [mlir][spirv] Split codegen for float min/max reductions and others (NFC) (PR #69023)

[Daniil Dudkin]

unterumarmung wrote:

> @unterumarmung I played with the code a bit and have an alternative idea: if all the derived classes implement just one function, could we make that function a constructor/template parameter and drop CRTP and the resulting complexity?

It is an interesting question!

In #69053, I avoided this complexity by just not refactoring. I began the refactoring because I believed it would be more challenging to implement the additional logic of the floating-point min/max operations. I did not want to complicate the main reduction pattern, especially in the macros. However, it turned out that all I needed to do was add one line to the macro, so I created #69053.

If you want to split the floating-point min/max ops from the rest, we could explore some options:
1. **Passing the function to the constructor**: It could work, but I don't have extensive experience with the MLIR infrastructure, so I'm not sure how we can introduce a constructor parameter here: https://github.com/llvm/llvm-project/blob/a736e6b5dfff09a8a24e17e130b3b8314ad8c825/mlir/lib/Conversion/VectorToSPIRV/VectorToSPIRV.cpp#L687-L697 
2. **Passing the function as a template parameter**: it seems easier than the first option. We could implement functions as `static` ones and simply pass pointers (or even `llvm::function_ref`s) to them into the template.
3. **Virtual functions**: we could implement the functions as virtual ones. It'd reduce the complexity of the template machinery by introducing a negligible runtime overhead.

There might be other options, but currently I can't think of any other.



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