Message ID | 6522a7d2-63a9-477a-a4b3-6069d83de0c0@arm.com |
---|---|
State | New |
Headers | show |
Series | Add a REG_P check for inc and dec for Arm MVE | expand |
Hi Saurabh, > -----Original Message----- > From: Saurabh Jha <saurabh.jha@arm.com> > Sent: Thursday, November 9, 2023 10:12 AM > To: gcc-patches@gcc.gnu.org; Richard Earnshaw > <Richard.Earnshaw@arm.com>; Richard Sandiford > <Richard.Sandiford@arm.com> > Subject: [PATCH] Add a REG_P check for inc and dec for Arm MVE > > Hey, > > This patch tightens mve_vector_mem_operand to reject non-register > operands inside {PRE,POST}_{INC,DEC} addresses by introducing a REG_P > check. > > This patch fixes this ICE:https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112337 > > Okay for trunk? I don't have trunk access so could someone please commit > on my behalf? Ok. > > Regards, > Saurabh > > gcc/ChangeLog: > > PR target/112337 > * config/arm/arm.cc (mve_vector_mem_operand): Add a REG_P > check for INC > and DEC operations > > gcc/testsuite/ChangeLog: > > PR target/112337 > * gcc.target/arm/mve/pr112337.c: Test for REG_P check for INC and > DEC > operations ChangeLog entries should end with a full stop (the git commit hooks enforce it). I've adjusted the ChangeLog and pushed this patch for you. Thank you for the patch! Kyrill
Hi Saurabh, On Tue, 14 Nov 2023 at 15:51, Kyrylo Tkachov <Kyrylo.Tkachov@arm.com> wrote: > > Hi Saurabh, > > > -----Original Message----- > > From: Saurabh Jha <saurabh.jha@arm.com> > > Sent: Thursday, November 9, 2023 10:12 AM > > To: gcc-patches@gcc.gnu.org; Richard Earnshaw > > <Richard.Earnshaw@arm.com>; Richard Sandiford > > <Richard.Sandiford@arm.com> > > Subject: [PATCH] Add a REG_P check for inc and dec for Arm MVE > > > > Hey, > > > > This patch tightens mve_vector_mem_operand to reject non-register > > operands inside {PRE,POST}_{INC,DEC} addresses by introducing a REG_P > > check. > > > > This patch fixes this ICE:https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112337 > > > > Okay for trunk? I don't have trunk access so could someone please commit > > on my behalf? > > Ok. > > > > > Regards, > > Saurabh > > > > gcc/ChangeLog: > > > > PR target/112337 > > * config/arm/arm.cc (mve_vector_mem_operand): Add a REG_P > > check for INC > > and DEC operations > > > > gcc/testsuite/ChangeLog: > > > > PR target/112337 > > * gcc.target/arm/mve/pr112337.c: Test for REG_P check for INC and > > DEC > > operations > This new test fails in our CI (various flavours of target arm-eabi), with the following: /gcc.target/arm/mve/pr112337.c:11:18: warning: passing argument 1 of '__arm_vldrwq_s32' from incompatible pointer type [-Wincompatible-pointer-types] In file included from /home/tcwg-buildslave/workspace/tcwg_gnu_0/abe/snapshots/gcc.git~master/gcc/testsuite/gcc.target/arm/mve/pr112337.c:5: /arm-eabi/14.0.0/include/arm_mve.h:1752:35: note: expected 'const int32_t *' {aka 'const long int *'} but argument is of type 'int *' I think you should make p a int32_t pointer rather than int, but I am wondering why you didn't see this problem in your testing? Can you check? Thanks, Christophe > ChangeLog entries should end with a full stop (the git commit hooks enforce it). > I've adjusted the ChangeLog and pushed this patch for you. > Thank you for the patch! > Kyrill >
Hi Christophe, On 11/20/2023 10:12 AM, Christophe Lyon wrote: > Hi Saurabh, > > On Tue, 14 Nov 2023 at 15:51, Kyrylo Tkachov <Kyrylo.Tkachov@arm.com> wrote: >> Hi Saurabh, >> >>> -----Original Message----- >>> From: Saurabh Jha <saurabh.jha@arm.com> >>> Sent: Thursday, November 9, 2023 10:12 AM >>> To: gcc-patches@gcc.gnu.org; Richard Earnshaw >>> <Richard.Earnshaw@arm.com>; Richard Sandiford >>> <Richard.Sandiford@arm.com> >>> Subject: [PATCH] Add a REG_P check for inc and dec for Arm MVE >>> >>> Hey, >>> >>> This patch tightens mve_vector_mem_operand to reject non-register >>> operands inside {PRE,POST}_{INC,DEC} addresses by introducing a REG_P >>> check. >>> >>> This patch fixes this ICE:https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112337 >>> >>> Okay for trunk? I don't have trunk access so could someone please commit >>> on my behalf? >> Ok. >> >>> Regards, >>> Saurabh >>> >>> gcc/ChangeLog: >>> >>> PR target/112337 >>> * config/arm/arm.cc (mve_vector_mem_operand): Add a REG_P >>> check for INC >>> and DEC operations >>> >>> gcc/testsuite/ChangeLog: >>> >>> PR target/112337 >>> * gcc.target/arm/mve/pr112337.c: Test for REG_P check for INC and >>> DEC >>> operations > This new test fails in our CI (various flavours of target arm-eabi), > with the following: > /gcc.target/arm/mve/pr112337.c:11:18: warning: passing argument 1 of > '__arm_vldrwq_s32' from incompatible pointer type > [-Wincompatible-pointer-types] > In file included from > /home/tcwg-buildslave/workspace/tcwg_gnu_0/abe/snapshots/gcc.git~master/gcc/testsuite/gcc.target/arm/mve/pr112337.c:5: > /arm-eabi/14.0.0/include/arm_mve.h:1752:35: note: expected 'const > int32_t *' {aka 'const long int *'} but argument is of type 'int *' > > I think you should make p a int32_t pointer rather than int, but I am > wondering why you didn't see this problem in your testing? > > Can you check? > > Thanks, > > Christophe Thank you for bringing this up. Unfortunately, I misread the test output in my local testing. Apologies for that. I am working on a fix and will be submitting it soon. Thanks, Saurabh > >> ChangeLog entries should end with a full stop (the git commit hooks enforce it). >> I've adjusted the ChangeLog and pushed this patch for you. >> Thank you for the patch! >> Kyrill >>
diff --git a/gcc/config/arm/arm.cc b/gcc/config/arm/arm.cc index 620ef7bfb2f..25a1ad736ad 100644 --- a/gcc/config/arm/arm.cc +++ b/gcc/config/arm/arm.cc @@ -13695,8 +13695,11 @@ mve_vector_mem_operand (machine_mode mode, rtx op, bool strict) } code = GET_CODE (op); - if (code == POST_INC || code == PRE_DEC - || code == PRE_INC || code == POST_DEC) + if ((code == POST_INC + || code == PRE_DEC + || code == PRE_INC + || code == POST_DEC) + && REG_P (XEXP (op, 0))) { reg_no = arm_effective_regno (XEXP (op, 0), strict); return (((mode == E_V8QImode || mode == E_V4QImode || mode == E_V4HImode) diff --git a/gcc/testsuite/gcc.target/arm/mve/pr112337.c b/gcc/testsuite/gcc.target/arm/mve/pr112337.c new file mode 100644 index 00000000000..8f491990088 --- /dev/null +++ b/gcc/testsuite/gcc.target/arm/mve/pr112337.c @@ -0,0 +1,15 @@ +/* { dg-do compile } */ +/* { dg-options "-O2" } */ +/* { dg-require-effective-target arm_v8_1m_mve_ok } */ +/* { dg-add-options arm_v8_1m_mve } */ +#include <arm_mve.h> + +void g(int32x4_t); +void f(int, int, int, short, int *p) { + int *bias = p; + for (;;) { + int32x4_t d = vldrwq_s32 (p); + bias += 4; + g(d); + } +}
Hey, This patch tightens mve_vector_mem_operand to reject non-register operands inside {PRE,POST}_{INC,DEC} addresses by introducing a REG_P check. This patch fixes this ICE:https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112337 Okay for trunk? I don't have trunk access so could someone please commit on my behalf? Regards, Saurabh gcc/ChangeLog: PR target/112337 * config/arm/arm.cc (mve_vector_mem_operand): Add a REG_P check for INC and DEC operations gcc/testsuite/ChangeLog: PR target/112337 * gcc.target/arm/mve/pr112337.c: Test for REG_P check for INC and DEC operations From a53553878602c7050b4f027a19149ca643b11721 Mon Sep 17 00:00:00 2001 From: Saurabh Jha <saujha01@e130340.arm.com> Date: Wed, 8 Nov 2023 14:33:35 +0000 Subject: [PATCH 1/1] Add a REG_P check for inc and dec for Arm MVE It is okay to have operands in MEM for inc and dec in Arm MVE. This patch introduces a REG_P check for inc and dec so that we don't get an ICE for the case of inc/dec on MEM. --- gcc/config/arm/arm.cc | 7 +++++-- gcc/testsuite/gcc.target/arm/mve/pr112337.c | 15 +++++++++++++++ 2 files changed, 20 insertions(+), 2 deletions(-) create mode 100644 gcc/testsuite/gcc.target/arm/mve/pr112337.c