On Tue, 31 Jul 2018 at 15:57, Segher Boessenkool <seg...@kernel.crashing.org> wrote: > > Hi Christophe, > > On Tue, Jul 31, 2018 at 02:34:06PM +0200, Christophe Lyon wrote: > > Since this was committed, I've noticed regressions > > on aarch64: > > FAIL: gcc.dg/zero_bits_compound-1.c scan-assembler-not \\(and: > > This went from > and w0, w0, 255 > lsl w1, w0, 8 > orr w0, w1, w0, lsl 20 > ret > to > and w1, w0, 255 > ubfiz w0, w0, 8, 8 > orr w0, w0, w1, lsl 20 > ret > so it's neither an improvement nor a regression, just different code. > The testcase wants no ANDs in the RTL.
I didn't try to manually regenerate the code before and after the patch, but if there was "and w0, w0, 255" before the patch, why did the test pass? > > on arm-none-linux-gnueabi > > FAIL: gfortran.dg/actual_array_constructor_1.f90 -O1 execution test > > That sounds bad. Open a PR, maybe? > I've just filed PR86771 > > On aarch64, I've also noticed a few others regressions but I'm not yet > > 100% sure it's caused by this patch (bisect running): > > gcc.target/aarch64/ashltidisi.c scan-assembler-times asr 4 > > ushift_53_i: > - uxtw x1, w0 > - lsl x0, x1, 53 > - lsr x1, x1, 11 > + lsr w1, w0, 11 > + lsl x0, x0, 53 > ret > > shift_53_i: > - sxtw x1, w0 > - lsl x0, x1, 53 > - asr x1, x1, 11 > + sbfx x1, x0, 11, 21 > + lsl x0, x0, 53 > ret > > Both are improvements afais. The number of asr insns changes, sure. > Looks like an easy "fix" would be to change to "scan-assembler-times asr 3" but maybe the aarch64 maintainers want to add more checks here (lsl/lsr counts) Don't you include arm/aarch64 in the 30 targets you used for testing? > > > gcc.target/aarch64/sve/var_stride_2.c -march=armv8.2-a+sve > > scan-assembler-times \\tadd\\tx[0-9]+, x[0-9]+, x[0-9]+, lsl 10\\n 2 > > Skipping all the SVE tests, sorry. Richard says they look like > improvements, and exactly of the expected kind. :-) > > > Segher
