Some of the lowering of vector.contract didn't support integer case. Since reduction of integer cannot accumulate we always break up the reduction op, it should be merged by a separate canonicalization if possible.
Details
Diff Detail
Event Timeline
mlir/lib/Dialect/Vector/VectorTransforms.cpp | ||
---|---|---|
1914 | Minor, but ignorable suggestion: since we repeat this, how about making a helper somewhere that is called createAdd and createMul and pass in the isInt? | |
mlir/test/Dialect/Vector/vector-contract-transforms.mlir | ||
21 | As you state in the description, we always have this breakup now, which is a bit unfortunate (the merged version tends to generate better code). Do you feel special cases codegen is too much of a maintenance burden? Can we at least get a commitment to get the follow up canonicalization in place |
mlir/test/Dialect/Vector/vector-contract-transforms.mlir | ||
---|---|---|
21 | For cases that reduce to FMA, I have one that I need to flush into core somewhere. |
mlir/lib/Dialect/Vector/VectorTransforms.cpp | ||
---|---|---|
1896 | Nit: You can move this to further below - to just outside the for loop. |
mlir/lib/Dialect/Vector/VectorTransforms.cpp | ||
---|---|---|
1896 | Done. | |
1914 | I was also thinking about it. I followed your suggestion. | |
mlir/test/Dialect/Vector/vector-contract-transforms.mlir | ||
21 | @nicolasvasilache pushed this canonicalization so this shouldn't be a concern! |
Nit: You can move this to further below - to just outside the for loop.