[llvm] [SLP] Avoid crash in computeExtractCost (PR #93188)
Alexey Bataev via llvm-commits
llvm-commits at lists.llvm.org
Thu May 23 06:34:11 PDT 2024
alexey-bataev wrote:
> Hi @alexey-bataev , do you think this patch looks OK despite not having any in-tree reproducer?
>
> I could not really find any matching piece of code doing the slicing based on EltsPerVector like that, but I wonder a bit if the cost estimates here need to match some other piece of code used to emit the actual shuffles?
>
> When I looked at this I also noticed some other things that looked suspect to me.
>
> ShuffleCostEstimator::adjustExtracts is calculating `SliceSize = VL.size() / NumParts`. But afaict this isn't always an exact division. For example test/Transforms/SLPVectorizer/AArch64/vec3-reorder-reshuffle.ll will end up in a situation when divide 3 by 2. And then
>
> ```
> for (unsigned Part = 0; Part < NumParts; ++Part) {
> ArrayRef<int> SubMask = Mask.slice(Part * SliceSize, SliceSize);
> for (auto [I, V] : enumerate(VL.slice(Part * SliceSize, SliceSize))) {
> ```
>
> won't cover for the full VL/mask. There are a couple of more places with similar code (e.g. BoUpSLP::tryToGatherExtractElements). It is not clear to me if this is a real problem. But the code seems to have some assumptions regarding how NumParts relates to VL.size() and Mask.size() etc. But it is not quite clear what those assumptions are.
adjustExtracts code is correct for power-of-2 number of elements, but not correct for non-power-of-2 number of elements. I will fix it.
https://github.com/llvm/llvm-project/pull/93188
More information about the llvm-commits
mailing list