On Mon, 8 Feb 2021 at 18:10, Kyrylo Tkachov via Gcc-patches <gcc-patches@gcc.gnu.org> wrote: > > > > > -----Original Message----- > > From: Andre Vieira (lists) <andre.simoesdiasvie...@arm.com> > > Sent: 03 February 2021 17:59 > > To: gcc-patches@gcc.gnu.org > > Cc: Kyrylo Tkachov <kyrylo.tkac...@arm.com> > > Subject: [AArch64] Fix vector multiplication costs > > > > This patch introduces a vect.mul RTX cost and decouples the vector > > multiplication costing from the scalar one. > > > > After Wilco's "AArch64: Add cost table for Cortex-A76" patch we saw a > > regression in vector codegen. Reproduceable with the small test added in > > this patch. > > Upon further investigation we noticed 'aarch64_rtx_mult_cost' was using > > scalar costs to calculate the cost of vector multiplication, which was > > now lower and preventing 'choose_mult_variant' from making the right > > choice to expand such vector multiplications with constants as shift and > > sub's. I also added a special case for SSRA to use the default vector > > cost rather than mult, SSRA seems to be cost using > > 'aarch64_rtx_mult_cost', which to be fair is quite curious. I believe we > > should have a better look at 'aarch64_rtx_costs' altogether and > > completely decouple vector and scalar costs. Though that is something > > that requires more rewriting than I believe should be done in Stage 4. > > > > I gave all targets a vect.mult cost of 4x the vect.alu cost, with the > > exception of targets with cost 0 for vect.alu, those I gave the cost 4. > > > > Bootstrapped on aarch64. > > > > Is this OK for trunk? > > Ok. > Thanks, > Kyrill > > > > > gcc/ChangeLog: > > > > * config/aarch64/aarch64-cost-tables.h: Add entries for vect.mul. > > * config/aarch64/aarch64.c (aarch64_rtx_mult_cost): Use > > vect.mul for > > vector multiplies and vect.alu for SSRA. > > * config/arm/aarch-common-protos.h (struct vector_cost_table): > > Define > > vect.mul cost field. > > * config/arm/aarch-cost-tables.h: Add entries for vect.mul. > > * config/arm/arm.c: Likewise. > > > > gcc/testsuite/ChangeLog: > > > > * gcc.target/aarch64/asimd-mul-to-shl-sub.c: New test. >
Hi Andre, It seems you forgot to update a test, because I've noticed these failures since you committed this patch: FAIL: gcc.target/aarch64/sve/mul_2.c -march=armv8.2-a+sve scan-assembler-times \\tmul\\tz[0-9]+\\.b, z[0-9]+\\.b, #-120\\n 1 FAIL: gcc.target/aarch64/sve/mul_2.c -march=armv8.2-a+sve scan-assembler-times \\tmul\\tz[0-9]+\\.h, z[0-9]+\\.h, #-128\\n 2 FAIL: gcc.target/aarch64/sve/mul_2.c -march=armv8.2-a+sve scan-assembler-times \\tmul\\tz[0-9]+\\.s, z[0-9]+\\.s, #-128\\n 1 Am I missing something? Thanks, Christophe
