On 24 November 2017 at 11:31, Tamar Christina <tamar.christ...@arm.com> wrote: >> > >> Not sure if Kyrill actually meant to comment about the three lines above, but >> they have a bug: >> #if should be before #pragma GCC push_options. >> >> Indeed, after this patch was committed (r255064), I've noticed many >> regressions, for instance >> p64_p128 is now unsupported. This is because the arm_crypto_ok effective >> target now fails with this message: >> XXX/arm_neon.h:16911:1: error: inlining failed in call to always_inline >> 'vaeseq_u8': target specific option mismatch >> >> Not sure why this wasn't noticed in validations earlier? > > I still have the log files for these runs: > > It seems that I was comparing the log files instead of the sum files, which > do not show this difference. > > /d/t/g/s/gcc (dot-product-arm ↩☡=) contrib/dg-cmp-results.sh -v -v "" > ../../build-arm-none-eabi/results.clean/vanilla/gcc.log > ../../build-arm-none-eabi/results.dotprod/vanilla/gcc.log | grep p64_p128 > /d/t/g/s/gcc (dot-product-arm ↩☡=) contrib/dg-cmp-results.sh -v -v "" > ../../build-arm-none-eabi/results.clean/vanilla/gcc.sum > ../../build-arm-none-eabi/results.dotprod/vanilla/gcc.sum | grep p64_p128 > NA->UNSUPPORTED: gcc.target/aarch64/advsimd-intrinsics/p64_p128.c -O0 > PASS->NA: gcc.target/aarch64/advsimd-intrinsics/p64_p128.c -O0 execution > test > PASS->NA: gcc.target/aarch64/advsimd-intrinsics/p64_p128.c -O0 (test for > excess errors) > NA->UNSUPPORTED: gcc.target/aarch64/advsimd-intrinsics/p64_p128.c -O1 > PASS->NA: gcc.target/aarch64/advsimd-intrinsics/p64_p128.c -O1 execution > test > PASS->NA: gcc.target/aarch64/advsimd-intrinsics/p64_p128.c -O1 (test for > excess errors) > NA->UNSUPPORTED: gcc.target/aarch64/advsimd-intrinsics/p64_p128.c -O2 > > Sorry for missing this, I don't even know why these scripts accept the log > files if they're always going to do the wrong thing. > > Anyway thanks for fixing this and I'll make sure I'm using the sum files in > the future. >
Thanks for checking why you missed it. That being said, I think there are a few more problems with your patch, but there is a lot of "noise" in the reports. 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) 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 >> >