[llvm] [LV] Add initial support for vectorizing literal struct return values (PR #109833)
Benjamin Maxwell via llvm-commits
llvm-commits at lists.llvm.org
Thu Nov 7 04:41:14 PST 2024
MacDue wrote:
I'm concerned that the multiple defs is not the correct approach, as it does not seem to mesh well with existing assumptions in the LV.
The current approach requires a few changes to allow the wide struct types. The largest is replacing `packScalarIntoVectorValue` with `packScalarIntoWideValue`, but other than that it mostly can reuse the existing code paths.
I've been trying to implement the multiple defs solution (and was able to get a minimal version working in #112402), but there are a few issues, and I'm not sure I see the benefits.
The two main issues I'm hitting are assumptions things are `VPSingleDefRecipe` and assumptions that instructions are scalarisable (even with an invalid cost). At least one of these needs to be fixed for a working implementation. I think the possible fixes are to make `VPReplicateRecipe` work for multiple defs, add a new a new recipe like `VPMultipleDefReplicateRecipe` (to avoid refactoring the single-def assumptions), or make vectorization when scalarization may happen illegal. That last one seemed a little awkward since in loop vectorization legality I don't have the `VF`, and later even with an `invalid` cost, if it's the only option it'll try to scalarize it.
I think this could be done, but I'm not sure we should model things this way. It conceptually makes sense, but even if implemented this way once you get to the cost model it's still expecting the actual return type and LLVM IR instruction in various places, so the abstraction feels somewhat leaky.
Given the original reason to try this was to avoid modifying `VPTransformState`, I'm not sure we'd still want to do this if it means a bunch more additional modifications to support multiple defs elsewhere.
I'd just like to make sure this makes sense because I'm hitting quite a few more issues than my original approach :sweat_smile:
https://github.com/llvm/llvm-project/pull/109833
More information about the llvm-commits
mailing list