Re: [PATCH][vectorizer][2/2] Hook up mult synthesis logic into vectorisation of mult-by-constant

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

Hi all,

On 07/07/16 17:16, Kyrill Tkachov wrote:
> On 06/07/16 13:40, Kyrill Tkachov wrote:
> > On 06/07/16 13:31, Rainer Orth wrote:
> > > Hi Kyrill,
> > >
> > > > On 05/07/16 12:24, Rainer Orth wrote:
> > > > > Marc Glisse <marc.glisse@inria.fr> writes:
> > > > >
> > > > > > On Tue, 5 Jul 2016, Kyrill Tkachov wrote:
> > > > > >
> > > > > > > As for testing I've bootstrapped and tested the patch on aarch64 and
> > > > > > > x86_64 with synth_shift_p in vect_synth_mult_by_constant hacked to be
> > > > > > > always true to exercise the paths that synthesize the shift by
> > > > > > > additions. Marc, could you test this on the sparc targets you care about?
> > > > > > I don't have access to Sparc, you want Rainer here (added in Cc:).
> > > > > As is, the patch doesn't even build on current mainline:
> > > > >
> > > > > /vol/gcc/src/hg/trunk/local/gcc/tree-vect-patterns.c:2151:56: error: 'mult_variant' has not been declared
> > > > >    target_supports_mult_synth_alg (struct algorithm *alg, mult_variant var,
> > > > > ^
> > > > >
> > > > > enum mult_variant is only declared in expmed.c.
> > > > Ah, sorry I forgot to mention that this is patch 2/2.
> > > > It requires https://gcc.gnu.org/ml/gcc-patches/2016-06/msg01144.html which
> > > > is already approved
> > > > but I was planning to commit it together with this one.
> > > > Can you please try applying
> > > > https://gcc.gnu.org/ml/gcc-patches/2016-06/msg01144.html
> > > > as well as this?
> > > sure, that did the trick.  A sparc-sun-solaris2.12 bootstrap revealed
> > > that the patch fixes PR tree-optimization/70923 (you should mention that
> > > in the ChangeLog or close it as a duplicate), with the same caveat as
> > > about Marc's latest patch for that:
> >
> > Thanks! Much appreciated.
> >
> > > +FAIL: gcc.dg/vect/vect-iv-9.c -flto -ffat-lto-objects  scan-tree-dump-times vect "vectorized 1 loops" 1
> > > +FAIL: gcc.dg/vect/vect-iv-9.c scan-tree-dump-times vect "vectorized 1 loops" 1
> > >
> > > The message appears twice, not once, on sparc, so the testcase should be
> > > updated to accomodate that.
> >
> > Ok.
> >
> > > Besides, the new testcase FAILs:
> > >
> > > +FAIL: gcc.dg/vect/pr65951.c -flto -ffat-lto-objects scan-tree-dump-times vect "vectorized 1 loops" 2
> > > +FAIL: gcc.dg/vect/pr65951.c scan-tree-dump-times vect "vectorized 1 loops" 2
> > >
> > > The dump contains
> > >
> > > gcc.dg/vect/pr65951.c:14:3: note: not vectorized: no vectype for stmt: _4 = *_3;
> > > gcc.dg/vect/pr65951.c:12:1: note: vectorized 0 loops in function.
> > > gcc.dg/vect/pr65951.c:21:3: note: not vectorized: no vectype for stmt: _4 = *_3;
> > > gcc.dg/vect/pr65951.c:19:1: note: vectorized 0 loops in function.
> > > gcc.dg/vect/pr65951.c:55:15: note: not vectorized: control flow in loop.
> > > gcc.dg/vect/pr65951.c:46:3: note: not vectorized: loop contains function calls or data references that cannot be analyzed
> > > gcc.dg/vect/pr65951.c:41:15: note: not vectorized: control flow in loop.
> > > gcc.dg/vect/pr65951.c:32:3: note: not vectorized: loop contains function calls or data references that cannot be analyzed
> > > gcc.dg/vect/pr65951.c:26:1: note: vectorized 0 loops in function.
> >
> > I see. I suppose SPARC doesn't have vector shifts operating on 64-bit data?
> > I believe the testcase should be updated to use just "int" arrays rather than "long long".
> >
> > I'll respin the testcases
>
> Ok, here's the patch with pr65951.c updated to use int rather than long long arrays and vect-iv-9.c updated to scan for the
> "Vectorized 1 loops" string twice. I've verified by hand by building a sparc-sun-solaris2.10 cc1 that compiling with
> -mcpu=ultrasparc -mvis gives the right strings in the vectoriser dump.
>
> So is this version ok?

So richi was ok with the implementation part of the patch [1]
Are the testuite changes made since ok?

[1] https://gcc.gnu.org/ml/gcc-patches/2016-07/msg00177.html
[2] https://gcc.gnu.org/ml/gcc-patches/2016-07/msg00331.html


Thanks,
Kyrill

> 2016-07-06  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>
>
>     PR target/65951
>     PR tree-optimization/70923
>     * tree-vect-patterns.c: Include mult-synthesis.h.
>     (target_supports_mult_synth_alg): New function.
>     (synth_lshift_by_additions): Likewise.
>     (apply_binop_and_append_stmt): Likewise.
>     (vect_synth_mult_by_constant): Likewise.
>     (target_has_vecop_for_code): Likewise.
>     (vect_recog_mult_pattern): Use above functions to synthesize vector
>     multiplication by integer constants.
>
> 2016-07-06  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>
>
>     PR target/65951
>     PR tree-optimization/70923
>     * gcc.dg/vect/vect-mult-const-pattern-1.c: New test.
>     * gcc.dg/vect/vect-mult-const-pattern-2.c: Likewise.
>     * gcc.dg/vect/pr65951.c: Likewise.
>     * gcc.dg/vect/vect-iv-9.c: Remove ! vect_int_mult-specific scan.