[PATCH] D100486: [COST]Improve cost model for shuffles in SLP.

Tue May 10 12:05:46 PDT 2022

asbirlea added a comment.

In D100486#3495174 <https://reviews.llvm.org/D100486#3495174>, @ABataev wrote:

> In D100486#3495157 <https://reviews.llvm.org/D100486#3495157>, @asbirlea wrote:
>
>> In D100486#3494746 <https://reviews.llvm.org/D100486#3494746>, @ABataev wrote:
>>
>>> In D100486#3494581 <https://reviews.llvm.org/D100486#3494581>, @asbirlea wrote:
>>>
>>>> We currently root-caused a 20% regression in eigen complex to this patch and it is blocking our compiler release.
>>>> I need some time to do additional testing, given there are at least a few patches on top of this. So far I see a crash fix, two NFCs and two additional changes that may also affect performance (https://reviews.llvm.org/D114171 and https://reviews.llvm.org/D115750).
>>>> Can you please hold off on any additional changes while we test further, or include them here for review so I can check if they resolve the regressions seen so far?
>>>
>>> Most probably, some overoptimistic cost model estimations turned on some extra vectorization. The reproducer will help, if you can provide it.
>>
>> The reproducer is open source: https://gitlab.com/libeigen/eigen
>> Regressions are on haswell platform, xfdo configuration; the compiler used is bootstrapped against itself; revision tested includes this patch and all follow-up changes to the SLPVectorizer including D114171 <https://reviews.llvm.org/D114171> and excluding D115750 <https://reviews.llvm.org/D115750>.
>> A few number samples:
>> BM_MatrixVectorMultiply_Complex_EigenRowMajorDynamic_float_16x64_ 16%
>> BM_MatrixVectorMultiply_Complex_EigenRowMajorDynamic_float_64x64_ 17%
>> BM_MatrixVectorMultiply_Complex_EigenRowMajorFixed_float_64x64_ 15%
>> BM_MatrixVectorMultiply_Complex_EigenRowMajorFixed_float_16x64_ 14%
>
> Could you at least extract the llvm ir code for one of these functions before the changes and after, so I coukd at least compare them?

Here are the only differences I see for dynamic, 64x64; base is before the patch, experiment includes this - revert+recommit -, https://reviews.llvm.org/rG371412e065a63107d5d79330da6757ff693d91cc,  https://reviews.llvm.org/D114171 and the NFCs.
Looks like the change is in the presence of poison:
F23023915: dumpslp_base_eigencomplex <https://reviews.llvm.org/F23023915>
F23023914: dumpslp_exp_eigencomplex <https://reviews.llvm.org/F23023914>

Another 30-40% regression on a non-public benchmark was root-caused to https://reviews.llvm.org/D114171, so investigating that.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100486