This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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


On Thu, 14 Jul 2016, Kyrill Tkachov wrote:

> 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

Ok with me - I wondered if Rainer wanted to double-check.

Richard.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]