Hi Christoph, > -----Original Message----- > From: Christophe Lyon [mailto:christophe.l...@linaro.org] > Sent: Sunday, November 26, 2017 20:01 > To: Tamar Christina <tamar.christ...@arm.com> > Cc: Kyrill Tkachov <kyrylo.tkac...@foss.arm.com>; gcc-patches@gcc.gnu.org; > nd <n...@arm.com>; Ramana Radhakrishnan > <ramana.radhakrish...@arm.com>; Richard Earnshaw > <richard.earns...@arm.com>; ni...@redhat.com > Subject: Re: [PATCH][GCC][ARM] Dot Product NEON intrinsics [Patch (3/8)] > > On 26 November 2017 at 13:56, Christophe Lyon <christophe.l...@linaro.org> > wrote: > > On 24 November 2017 at 20:38, Christophe Lyon > > <christophe.l...@linaro.org> wrote: > >> On 24 November 2017 at 19:05, Tamar Christina > <tamar.christ...@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 > >>>> >> >
