[PATCH] D36130: [SLP] Vectorize jumbled memory loads.

[Evgeny Astigeevich via llvm-commits]

Hi Shahid,

Thank you for fixing the issue. I also want to apologise if I used a wrong name. I had a problem to find a correct one.

I agree with you, there is no mandatory requirement to test and to benchmark all targets. We usually monitor the llvm mailing lists and try to check performance impact of patches.
Unfortunately the patch D36130 was not sent to llvm-commits and we did not  see it.
On llvm-dev we have been asked many times to assess impact on ARM targets even the patch was not related to ARM at all. We always welcome this.
Regarding the patch D36130, it has found several issues in different places to fix.

BTW, the regressed benchmarks are the part of LLVM performance testing: http://lnt.llvm.org/ . There are also x86 runs.

Thanks,
Evgeny

On 16/12/2017, 12:03, "Shahid, Asghar-ahmad" <Asghar-ahmad.Shahid at amd.com> wrote:

    Hi Evgeny,
    
    My bad, I did miss to include the profitability check for extra shuffle which I thought is included as it was present in https://reviews.llvm.org/D26905
    
    I will send a patch soon to fix this.
    
    Regards,
    Shahid
    
    > -----Original Message-----
    > From: Shahid, Asghar-ahmad
    > Sent: Saturday, December 16, 2017 1:23 PM
    > To: 'Evgeny Astigeevich' <Evgeny.Astigeevich at arm.com>
    > Cc: mgrang at codeaurora.org; diego.caballero at intel.com;
    > hans at chromium.org; mzolotukhin at apple.com; anna at azul.com; Florian
    > Hahn <Florian.Hahn at arm.com>; keno at juliacomputing.com;
    > mkuper at google.com;
    > reviews+D36130+public+cceac88dc376ed77 at reviews.llvm.org; Renato Golin
    > <renato.golin at linaro.org>; Kristof Beyls <Kristof.Beyls at arm.com>;
    > t.p.northover at gmail.com; danielcdh at gmail.com; zvi.rackover at intel.com;
    > ayal.zaks at intel.com; nd <nd at arm.com>
    > Subject: RE: [PATCH] D36130: [SLP] Vectorize jumbled memory loads.
    > 
    > Hi Evgeny,
    > 
    > I am not aware that it is mandatory to add tests or do the performance
    > assessment for each and every targets. OTOH, the check for profitability
    > happens w.r.t to the built vectorizable tree. So it seems the regressing tests
    > for your target think it is profitable to vectorize.
    > 
    > Hence, IMO, the issue seems to be either in the cost-modelling or the
    > subsequent lowering for the given target.
    > 
    > Regards,
    > Shahid
    > 
    > > -----Original Message-----
    > > From: Evgeny Astigeevich [mailto:Evgeny.Astigeevich at arm.com]
    > > Sent: Friday, December 15, 2017 10:56 PM
    > > To: Shahid, Asghar-ahmad <Asghar-ahmad.Shahid at amd.com>
    > > Cc: mgrang at codeaurora.org; diego.caballero at intel.com;
    > > hans at chromium.org; mzolotukhin at apple.com; anna at azul.com; Florian
    > Hahn
    > > <Florian.Hahn at arm.com>; keno at juliacomputing.com;
    > mkuper at google.com;
    > > reviews+D36130+public+cceac88dc376ed77 at reviews.llvm.org; Renato
    > Golin
    > > <renato.golin at linaro.org>; Kristof Beyls <Kristof.Beyls at arm.com>;
    > > t.p.northover at gmail.com; danielcdh at gmail.com; zvi.rackover at intel.com;
    > > ayal.zaks at intel.com; nd <nd at arm.com>
    > > Subject: Re: [PATCH] D36130: [SLP] Vectorize jumbled memory loads.
    > >
    > > Hi Mohammad,
    > >
    > > I created a problem report:
    > > https://bugs.llvm.org/show_bug.cgi?id=35673
    > >
    > > I think the patch has the following issues:
    > >
    > > 1. No assessment of performance impact on other targets.
    > > 2. No tests for other targets. I see it's tested only for x86.
    > > 3. In the patch I don't see any checks if the optimization is profitable.
    > >
    > > What if we revert the patch to fix these issues?
    > >
    > > Thanks,
    > > Evgeny Astigeevich
    > >
    > > On 14/12/2017, 13:32, "Evgeny Astigeevich via Phabricator"
    > > <reviews at reviews.llvm.org> wrote:
    > >
    > >     eastig added a comment.
    > >
    > >     Hi Shahid,
    > >
    > >     These changes caused 27.7% and 30.2% regressions on an AArch64
    > > Juno board (http://lnt.llvm.org/db_default/v4/nts/83681):
    > >
    > >     MultiSource/Benchmarks/mediabench/gsm/toast/toast: 30.20%
    > >     MultiSource/Benchmarks/MiBench/telecomm-gsm/telecomm-gsm:
    > 27.73%
    > >
    > >     We have the same benchmarks regressed on our AArch64 boards
    > > (Cortex- A53, Cortex-A57).
    > >
    > >     -Evgeny Astigeevich
    > >     The ARM Compiler Optimisation team
    > >
    > >
    > >     https://reviews.llvm.org/D36130
    > >
    > >
    > >
    > >