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

Nirav Dave via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Jun 18 05:53:51 PDT 2019


niravd added a comment.

> Hi, @niravd, after investigate code carefully, I think this check might not be redundant. Considering the case, we have 3 i32 values extracted from vectors, both `isTypeLegal` and `TIL.isTypeLegal` see a `v3i32` illegal.

Are you certain isTypeLegal was returning false in prelegalization?  isTypeLegal(x) should be "!LegalTypes || TLI.isTypeLegal(x)". I was expecting that if you swapped out TLI.isTypeLegal for isTypeLegal where it was failing, we could generate an invalid node, but it sounds like that's not the case. If so, I think we should double check.

> However, `MergeStoresOfConstantsOrVecElts` which is called later by `MergeConsecutiveStores` doesn't require type legality and builds a `BUILD_VECTOR` node whose elements are 3 `EXTRACT_VECTOR_ELT` values. PowerPC's vector type legalizer can handle such cases, so it can benefit from `getNumStoresOfVectorElementsToMergePreLegalize`. I know it's quite weird such check added within a context where type legality check is around. I once wanna implement it in `PPCTargetLowering::PerformDAGCombine`, however it might make code duplicated.




Repository:
  rL LLVM

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

https://reviews.llvm.org/D62890





More information about the llvm-commits mailing list