[PATCH] D62890: [DAGCombiner] Merge consecutive stores of vector elements before types are legalized

[Nirav Dave via Phabricator via llvm-commits]

niravd added a comment.

It's reasonable assuming we want to prohibit the SystemZ cases. Note that  TLI.canMergeConsecutiveStoresOfVectorElements and TLI.canMergeStoresTo are both only called here and should be merged into a single method.

That said, the permute expression seems like a sign that there are some permutation peepholes may be worthwhile. We could simplify the permute into a BUILD_VECTOR of a vector stores which should be close enough to the original element-wise stores here.

In D62890#1557119 <https://reviews.llvm.org/D62890#1557119>, @lkail wrote:

> Considering suggestions of @niravd and @uweigand , is it proper to have an implementation like
>
>   bool DAGCombiner::isAbleToMergeConsecutiveStoresPreLegalize(ArrayRef<SNode*> elements, .../* Params related to align, addrspace and etc.*/) {
>     if (LegalTypes)
>       return false;
>     // Let target decides cost considering elements to be stored.
>     return TLI.canMergeConsecutiveStoresOfVectorElements(elements, ...);
>   }
>
>
> And new check is
>
>   if (isTypeLegal(Ty) &&
>       TLI.canMergeStoresTo(FirstStoreAS, Ty, DAG) &&
>       ((TLI.allowsMemoryAccess(Context, DL, Ty,
>                              *FirstInChain->getMemOperand(), &IsFast) &&
>         IsFast) || isAbleToMergeConsecutiveStoresPreLegalize(elements, ...))
>   





Repository:
  rL LLVM

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

https://reviews.llvm.org/D62890