[PATCH] D24683: [DAGCombine] Generalize build_vector -> vector_shuffle combine for more than 2 inputs

[Simon Pilgrim via llvm-commits]

RKSimon added a comment.

In https://reviews.llvm.org/D24683#545671, @mkuper wrote:

> In https://reviews.llvm.org/D24683#545621, @RKSimon wrote:
>
> > Nice work! There's a lot going on here which makes we wonder whether this should be split into several patches:
> >
> > 1 - createBuildVecShuffle - generalise support for the shuffling of different sized inputs + outputs
> >  2 - shuffling with constant values (not just zeros)
> >  3 - support any number of inputs
>
>
> Thanks, Simon!
>
> I'm not sure what you mean by "split". :-)
>  This patch only contains item 3.
>
> Re 1 - splitting the shuffle logic into "createBuildVecShuffle" doesn't generalize anything, it's just a refactoring, that only makes sense once we have more than 2 inputs. The restrictions on input sizes existed before this patch.
>  Re 2 - we have only been blending with zeroes for years. I agree we should probably blend with any constant vectors, and will be happy to add yet another TODO. However, that seems orthogonal to this patch.
>
> In any case, I don't see a reason to gate (3) on either (1) or (2).


Sorry for not being clear, but yes my concern was whether the createBuildVecShuffle refactor should be done first as it would make the rest of the patch easier to follow. My proposal for the order of work was based on trying to reduce the size of each patch and show the effect of each feature addition. I can understand if you don't want to take that approach though.


https://reviews.llvm.org/D24683