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

[Alexey Bataev via Phabricator via llvm-commits]

ABataev added a comment.

In D100486#3504277 <https://reviews.llvm.org/D100486#3504277>, @asbirlea wrote:

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

Hmm, I don't see the difference, actually, just changed the order of evaluation and lanes switched, no actual difference in IR. Most probably, the regression is caused by something else, maybe some lowering passes were tweaked for the previous order but not for the current.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100486