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-patches/255063-r255064-fixed.patch/report-build-info.html 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-patches/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. 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..90b00aff95cfef96d1963be176 >>> >> 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 >>> >> >
