Re: [PATCH][expmed] Properly account for the cost and latency of shift+add ops when synthesizing mults

[Kyrill Tkachov <kyrylo dot tkachov at arm dot com>]

Hi Jeff,

On 17/04/15 20:38, Jeff Law wrote:
> On 04/14/2015 02:11 AM, Kyrill Tkachov wrote:
> > Of course the effect on codegen of this patch depends a lot on the rtx
> > costs in the backend.
> > On aarch64 with -mcpu=cortex-a57 tuning I see the cost limit being
> > exceeded in more cases and the
> > expansion code choosing instead to do a move-immediate and a mul
> > instruction.
> > No regressions on SPEC2006 on a Cortex-A57.
> >
> > For example, for code:
> > long f0 (int x, int y)
> > {
> >     return (long)x * 6L;
> > }
> >
> >
> > int f1(int x)
> > {
> >     return x * 10;
> > }
> >
> > int f2(int x)
> > {
> >       return x * 100;
> > }
> >
> > int f3(int x)
> > {
> >       return x * 20;
> > }
> >
> > int f4(int x)
> > {
> >       return x * 25;
> > }
> >
> > int f5(int x)
> > {
> >         return x * 11;
> > }
> Please turn this into a test for the testsuite.  It's fine if this the
> test is specific to AArch64.  You may need to break it into 6 individual
> tests since what you want to check for in each one may be significantly
> different.  For example, f0, f4 and f5 you'd probably check for the
> constant load & multiply instructions.  Not sure how to best test for
> what you want in f1-f3.

f1/f3 still end up synthesising the mult, but prefer a different
algorithm. I don't think the algorithm chosen in f1/f3 is worse or
better than what it was producing before, so I don't think there's
much point in testing for it. If you think it's really better to
test for something, I propose testing that only two instructions are
generated, and neither of them are a 'mul'. I'll repost a patch with
my proposed testcases for f0,f2,f4,f5.

> > Bootstrapped and tested on arm, aarch64, x86_64-linux.
> > Ok for trunk?
> >
> > Thanks,
> > Kyrill
> >
> > 2015-04-14  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>
> >
> >       * expmed.c: (synth_mult): Only assume overlapping
> >       shift with previous steps in alg_sub_t_m2 case.
> OK with a test for the testsuite.

Thanks for looking at this,
Kyrill


> jeff