[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