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

Kai Luo via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Jun 17 01:27:28 PDT 2019


lkail added a comment.

In D62890#1532667 <https://reviews.llvm.org/D62890#1532667>, @niravd wrote:

> This seems like it should be folded into the already existing checks. Do you know why NumStoresToMerge was not being before. I expect it's the requirement of a legal types in pre-legal merges or PPC's check for allowed misaligned accesses missing some cases. If it's the former I suspect we can disable the legality requirement prelegaltypes for non-truncated stores (replace TLI.isTypeLegal(ty) with isTypeLegal(ty)).


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. 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