[PATCH] D148523: [LegalizeTypes][VP] Widen fixed length vectors to VP nodes

Wed Apr 19 04:41:45 PDT 2023

luke added a comment.

In D148523#4278108 <https://reviews.llvm.org/D148523#4278108>, @craig.topper wrote:

> Can we split load/stores to their own patch? I'm happy to approve those.

Of course, will do.

In D148523#4278114 <https://reviews.llvm.org/D148523#4278114>, @craig.topper wrote:

> I'm a little concerned about the lack of DAG combines on VP nodes. Will using VP prevent optimizations?

I believe so. Long term I presume we'll have to flesh out https://reviews.llvm.org/D141891 to cover every combine under the sun, which sounds like a lot of work. (This is the "Incrementally lift DAGCombiner to work on VP SDNodes as well as on regular vector instructions." step of the VP roadmap <https://llvm.org/docs/Proposals/VectorPredication.html>)
(cc @fakepaper56 do you have any other plans to extend the use of VPMatchContext?)

I'll look into the quality of non-combined-VP-widened code vs combined-but-non-VP-widened code in the wild and get back to you on this.

As a side note, I think loads and stores might be the some of the most affected, since `FindBetterChain` and friends in DAGCombine would need to be updated to reason about aliasing etc. in the presence of VP nodes.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D148523/new/

https://reviews.llvm.org/D148523