[PATCH][GCC][ARM] Dot Product NEON intrinsics [Patch (3/8)]

Tamar Christina Tamar.Christina@arm.com
Mon Nov 27 11:14:00 GMT 2017


Hi Christoph,

> -----Original Message-----
> From: Christophe Lyon [mailto:christophe.lyon@linaro.org]
> Sent: Sunday, November 26, 2017 20:01
> To: Tamar Christina <Tamar.Christina@arm.com>
> Cc: Kyrill Tkachov <kyrylo.tkachov@foss.arm.com>; gcc-patches@gcc.gnu.org;
> nd <nd@arm.com>; Ramana Radhakrishnan
> <Ramana.Radhakrishnan@arm.com>; Richard Earnshaw
> <Richard.Earnshaw@arm.com>; nickc@redhat.com
> Subject: Re: [PATCH][GCC][ARM] Dot Product NEON intrinsics [Patch (3/8)]
> 
> On 26 November 2017 at 13:56, Christophe Lyon <christophe.lyon@linaro.org>
> wrote:
> > On 24 November 2017 at 20:38, Christophe Lyon
> > <christophe.lyon@linaro.org> wrote:
> >> On 24 November 2017 at 19:05, Tamar Christina
> <Tamar.Christina@arm.com> wrote:
> >>> Hi Christophe,
> >>>
> >>>>
> >>>> After your commit, I have these reports:
> >>>> http://people.linaro.org/~christophe.lyon/cross-
> >>>> validation/gcc/trunk/255064/report-build-info.html
> >>>>
> >>>> After my commit, I have these reports:
> >>>> http://people.linaro.org/~christophe.lyon/cross-
> >>>> validation/gcc/trunk/255126/report-build-info.html
> >>>>
> >>>> I haven't fully checked that my patch fixes all the regressions
> >>>> reported at r255064, but I don't see why my patch would introduce
> >>>> regressions.... So I think your patch is causing problems:
> >>>> * on armeb --with-fpu=neon-fp16: (the 2 "REGRESSED" entries):
> >>>>     gcc.target/arm/attr-neon3.c scan-assembler-times vld1 1 (found 2
> times)
> >>>>     gcc.target/arm/neon-vfma-1.c scan-assembler vfma\\.f32[\t]+[dDqQ]
> >>>>     gcc.target/arm/neon-vfms-1.c scan-assembler
> >>>> vfms\\.f32[\t]+[dDqQ]
> >>>>
> >>>> * on arm-none-linux-gnueabihf --with-cpu cortex-a5 --with-fpu
> >>>> vfpv3-d16-
> >>>> fp16 and armeb-none-linux-gnueabihf --with-cpu cortex-a9 --with-fpu
> >>>> vfpv3-
> >>>> d16-fp16 (the 2 "BIG-REGR" entries)
> >>>
> >>> This patch only introduced a few neon instrinsics in arm_neon.h, and
> most of these files don't use the header.
> >>>
> >>> gcc.dg/vect/pr65947-14.c doesn't exist in my tree so it's a relatively new
> test.
> >>>
> >>> I will run some regressions over the weekend on an updated tree, but
> >>> I can't understand how a not included header it can cause execution
> >>> failures 😊
> >>> However most of those are vectorizer tests. It seems much more likely
> to me that vectorization is broken rather.
> >>
> >> Agreed. But note that many regressions are reported for the
> >> configurations --with-fpu vfpv3-d16-fp16
> >> at:
> >> http://people.linaro.org/~christophe.lyon/cross-validation/gcc/trunk/
> >> 255064/report-build-info.html Maybe that's just a matter of
> >> arm_neon.h being included by some effective-target tests?
> >>
> >>
> > Hi Tamar,
> >
> > Good news, I have confirmed your obvious thoughts: I have run
> > validations of r255063+your patch fixed, and the results are clean:
> > http://people.linaro.org/~christophe.lyon/cross-validation/gcc-test-pa
> > tches/255063-r255064-fixed.patch/report-build-info.html

Thanks for confirming! My own finished as well. Sorry again for the breakage, I've
Updated my scripts to exclude log files so this shouldn't happen again!.


> >
> > I have also compared r255063 to r255216 (that is I applied all patches
> > between yours and mine):
> > http://people.linaro.org/~christophe.lyon/cross-validation/gcc-test-pa
> > tches/255063-r255063-255126.patch/report-build-info.html
> > which confirms some regressions have been introduced in-between,
> > hidden by the problem in your patch.
> >
> > Some may be obvious to bisect, some less.
> >
> OK, so for gcc:
> FAIL: gcc.dg/ipa/inline-1.c scan-ipa-dump inline "op2 change 9.990000. of
> time"
> after r255103, which updated the test
> 
> several failures for gcc.target/arm/addr-modes-float.c which was introduced
> at r255111 (Charles is aware of that, probably just a matter of adding the right
> effective-target)
> 
> I'm still trying to reproduce the regression:
> FAIL: gcc.dg/vect/vect-nb-iter-ub-2.c execution test on armeb
> 
> and for g++:
> g++.dg/ipa/devirt-22.C  -std=gnu++11  scan-ipa-dump-times cp
> "Discovered a virtual call to a known target" 1 (found 2 times)
> g++.dg/ipa/devirt-22.C  -std=gnu++14  scan-ipa-dump-times cp
> "Discovered a virtual call to a known target" 1 (found 2 times)
> g++.dg/ipa/devirt-22.C  -std=gnu++98  scan-ipa-dump-times cp
> "Discovered a virtual call to a known target" 1 (found 2 times)
> g++.dg/pr79095-4.C  -std=gnu++98  scan-tree-dump-times vrp2
> "__builtin_memset \\(_[0-9]+, 0, [0-9]+\\)" 1 (found 0 times)
> g++.dg/pr79095-4.C  -std=gnu++98  (test for warnings, line )
> 
> Christophe
> 
> 
> > Christophe
> >
> >>>
> >>> Thanks,
> >>> Tamar
> >>>
> >>>> where a few tests fail:
> >>>> (arm-none-linux-gnueabihf cortex-a5 vfpv3-d16-fp16):
> >>>>     gcc.dg/vect/pr65947-14.c -flto -ffat-lto-objects execution test
> >>>>     gcc.dg/vect/pr65947-14.c execution test
> >>>>
> >>>> (armeb-none-linux-gnueabihf cortex-a9 vfpv3-d16-fp16):
> >>>>   Executed from: gcc.dg/vect/vect.exp
> >>>>     gcc.dg/vect/pr51074.c -flto -ffat-lto-objects execution test
> >>>>     gcc.dg/vect/pr51074.c execution test
> >>>>     gcc.dg/vect/pr64252.c -flto -ffat-lto-objects execution test
> >>>>     gcc.dg/vect/pr65947-14.c -flto -ffat-lto-objects execution test
> >>>>     gcc.dg/vect/pr65947-14.c execution test
> >>>>     gcc.dg/vect/vect-cond-4.c -flto -ffat-lto-objects execution test
> >>>>     gcc.dg/vect/vect-nb-iter-ub-2.c execution test
> >>>>     gcc.dg/vect/vect-nb-iter-ub-3.c -flto -ffat-lto-objects execution test
> >>>>     gcc.dg/vect/vect-nb-iter-ub-3.c execution test
> >>>>     gcc.dg/vect/vect-strided-shift-1.c -flto -ffat-lto-objects execution
> test
> >>>>     gcc.dg/vect/vect-strided-shift-1.c execution test
> >>>>     gcc.dg/vect/vect-strided-u16-i3.c -flto -ffat-lto-objects execution test
> >>>>     gcc.dg/vect/vect-strided-u16-i3.c execution test
> >>>>   Executed from: gcc.target/arm/arm.exp
> >>>>     gcc.target/arm/attr-neon3.c scan-assembler-times vld1 1 (found 2
> times)
> >>>>     gcc.target/arm/neon-vfma-1.c scan-assembler vfma\\.f32[\t]+[dDqQ]
> >>>>     gcc.target/arm/neon-vfms-1.c scan-assembler vfms\\.f32[\t]+[dDqQ]
> >>>>     gcc.target/arm/neon-vmla-1.c scan-assembler vmla\\.i32
> >>>>     gcc.target/arm/neon-vmls-1.c scan-assembler vmls\\.i32
> >>>>     gcc.target/arm/vect-copysignf.c scan-tree-dump-times vect
> >>>> "vectorized 1 loops" 1 (found 0 times)
> >>>>
> >>>> I haven't checked whether this tests were already failing before
> >>>> your patch, and are just reported as new failures because they
> >>>> failed to compile in the mean time.
> >>>>
> >>>> Not sure I am clear :-)
> >>>>
> >>>> Sorry for the delay and potentially hard to parse reports, I'm
> >>>> struggling with infrastructure problems.
> >>>>
> >>>> Thanks,
> >>>>
> >>>> Christophe
> >>>>
> >>>> > Tamar
> >>>> >
> >>>> >>
> >>>> >> Fixed as obvious (r255126).
> >>>> >>
> >>>> >> Christophe
> >>>> >>
> >>>> >> > diff --git a/gcc/testsuite/gcc.target/arm/simd/vect-dot-qi.h
> >>>> >> > b/gcc/testsuite/gcc.target/arm/simd/vect-dot-qi.h
> >>>> >> > new file mode 100644
> >>>> >> > index
> >>>> >> >
> >>>> >>
> >>>>
> 0000000000000000000000000000000000000000..90b00aff95cfef96d1963be17
> >>>> 6
> >>>> >> 73
> >>>> >> > dc191cc71169
> >>>> >> > --- /dev/null
> >>>> >> > +++ b/gcc/testsuite/gcc.target/arm/simd/vect-dot-qi.h
> >>>> >> > @@ -0,0 +1,15 @@
> >>>> >> > +TYPE char X[N] __attribute__
> >>>> >> ((__aligned__(__BIGGEST_ALIGNMENT__)));
> >>>> >> > +TYPE char Y[N] __attribute__
> >>>> >> ((__aligned__(__BIGGEST_ALIGNMENT__)));
> >>>> >> > +
> >>>> >> > +__attribute__ ((noinline)) int foo1(int len) {
> >>>> >> > +  int i;
> >>>> >> > +  TYPE int result = 0;
> >>>> >> > +  TYPE short prod;
> >>>> >> > +
> >>>> >> > +  for (i=0; i<len; i++) {
> >>>> >> > +    prod = X[i] * Y[i];
> >>>> >> > +    result += prod;
> >>>> >> > +  }
> >>>> >> > +  return result;
> >>>> >> > +}
> >>>> >> > \ No newline at end of file
> >>>> >> >
> >>>> >> > Please add new lines at the end of the new test files.
> >>>> >> > This applies to a few more new files in this patch.
> >>>> >> >
> >>>> >> > Ok with these nits fixed.
> >>>> >> >
> >>>> >> > Thanks,
> >>>> >> > Kyrill
> >>>> >> >


More information about the Gcc-patches mailing list