From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by sourceware.org (Postfix) with ESMTP id 3E1B9394741B for ; Mon, 5 Dec 2022 18:38:40 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 3E1B9394741B Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=arm.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=arm.com Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 4246B23A; Mon, 5 Dec 2022 10:38:46 -0800 (PST) Received: from localhost (e121540-lin.manchester.arm.com [10.32.99.50]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 1921F3F73B; Mon, 5 Dec 2022 10:38:38 -0800 (PST) From: Richard Sandiford To: Prathamesh Kulkarni Mail-Followup-To: Prathamesh Kulkarni ,gcc Patches , richard.sandiford@arm.com Cc: gcc Patches Subject: Re: [aarch64] PR107920 - Fix incorrect handling of virtual operands in svld1rq_impl::fold References: Date: Mon, 05 Dec 2022 18:38:37 +0000 In-Reply-To: (Prathamesh Kulkarni's message of "Fri, 2 Dec 2022 12:51:37 +0530") Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.3 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-Spam-Status: No, score=-39.0 required=5.0 tests=BAYES_00,GIT_PATCH_0,KAM_DMARC_NONE,KAM_DMARC_STATUS,KAM_LAZY_DOMAIN_SECURITY,KAM_SHORT,SPF_HELO_NONE,SPF_NONE,TXREP,WEIRD_PORT autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: Prathamesh Kulkarni writes: > Hi, > The following test: > > #include "arm_sve.h" > > svint8_t > test_s8(int8_t *x) > { > return svld1rq_s8 (svptrue_b8 (), &x[0]); > } > > ICE's with -march=3Darmv8.2-a+sve -O1 -fno-tree-ccp -fno-tree-forwprop: > during GIMPLE pass: fre > pr107920.c: In function =E2=80=98test_s8=E2=80=99: > pr107920.c:7:1: internal compiler error: in execute_todo, at passes.cc:21= 40 > 7 | } > | ^ > 0x7b03d0 execute_todo > ../../gcc/gcc/passes.cc:2140 > > because of incorrect handling of virtual operands in svld1rq_impl::fold: > # VUSE <.MEM> > _5 =3D MEM [(signed char * {ref-all})x_3(D)]; > _4 =3D VEC_PERM_EXPR <_5, _5, { 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, > 12, 13, 14, 15, ... }>; > # VUSE <.MEM_2(D)> > return _4; > > The attached patch tries to fix the issue by building the replacement > statements in gimple_seq, and passing it to gsi_replace_with_seq_vops, > which resolves the ICE, and results in: > : > # VUSE <.MEM_2(D)> > _5 =3D MEM [(signed char * {ref-all})x_3(D)]; > _4 =3D VEC_PERM_EXPR <_5, _5, { 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, > 12, 13, 14, 15, ... }>; > # VUSE <.MEM_2(D)> > return _4; > > Bootstrapped+tested on aarch64-linux-gnu. > OK to commit ? Looks good, but we also need to deal with the -fnon-call-exceptions point that Andrew made in the PR trail. An easy way of testing would be to make sure that: #include "arm_sve.h" svint8_t test_s8(int8_t *x) { try { return svld1rq_s8 (svptrue_b8 (), &x[0]); } catch (...) { return svdup_s8 (1); } } compiled with -fnon-call-exceptions still has a call to __cxa_begin_catch. I don't think it's worth optimising this case. Let's just add !flag_non_call_exceptions to the test. The patch is missing a changelog btw. Thanks, Richard > Thanks, > Prathamesh > > diff --git a/gcc/config/aarch64/aarch64-sve-builtins-base.cc b/gcc/config= /aarch64/aarch64-sve-builtins-base.cc > index 6347407555f..f5546a65d22 100644 > --- a/gcc/config/aarch64/aarch64-sve-builtins-base.cc > +++ b/gcc/config/aarch64/aarch64-sve-builtins-base.cc > @@ -45,6 +45,7 @@ > #include "aarch64-sve-builtins-base.h" > #include "aarch64-sve-builtins-functions.h" > #include "ssa.h" > +#include "gimple-fold.h" >=20=20 > using namespace aarch64_sve; >=20=20 > @@ -1232,7 +1233,9 @@ public: > tree mem_ref_op =3D fold_build2 (MEM_REF, access_type, arg1, zero); > gimple *mem_ref_stmt > =3D gimple_build_assign (mem_ref_lhs, mem_ref_op); > - gsi_insert_before (f.gsi, mem_ref_stmt, GSI_SAME_STMT); > + > + gimple_seq stmts =3D NULL; > + gimple_seq_add_stmt_without_update (&stmts, mem_ref_stmt); >=20=20 > int source_nelts =3D TYPE_VECTOR_SUBPARTS (access_type).to_constant (); > vec_perm_builder sel (lhs_len, source_nelts, 1); > @@ -1245,8 +1248,11 @@ public: > indices)); > tree mask_type =3D build_vector_type (ssizetype, lhs_len); > tree mask =3D vec_perm_indices_to_tree (mask_type, indices); > - return gimple_build_assign (lhs, VEC_PERM_EXPR, > - mem_ref_lhs, mem_ref_lhs, mask); > + gimple *g2 =3D gimple_build_assign (lhs, VEC_PERM_EXPR, > + mem_ref_lhs, mem_ref_lhs, mask); > + gimple_seq_add_stmt_without_update (&stmts, g2); > + gsi_replace_with_seq_vops (f.gsi, stmts); > + return g2; > } >=20=20 > return NULL; > diff --git a/gcc/gimple-fold.cc b/gcc/gimple-fold.cc > index c2d9c806aee..03cdb2f9f49 100644 > --- a/gcc/gimple-fold.cc > +++ b/gcc/gimple-fold.cc > @@ -591,7 +591,7 @@ fold_gimple_assign (gimple_stmt_iterator *si) > If the statement has a lhs the last stmt in the sequence is expected > to assign to that lhs. */ >=20=20 > -static void > +void > gsi_replace_with_seq_vops (gimple_stmt_iterator *si_p, gimple_seq stmts) > { > gimple *stmt =3D gsi_stmt (*si_p); > diff --git a/gcc/gimple-fold.h b/gcc/gimple-fold.h > index 7d29ee9a9a4..87ed4e56d25 100644 > --- a/gcc/gimple-fold.h > +++ b/gcc/gimple-fold.h > @@ -63,6 +63,7 @@ extern bool arith_code_with_undefined_signed_overflow (= tree_code); > extern gimple_seq rewrite_to_defined_overflow (gimple *, bool =3D false); > extern void replace_call_with_value (gimple_stmt_iterator *, tree); > extern tree tree_vec_extract (gimple_stmt_iterator *, tree, tree, tree, = tree); > +extern void gsi_replace_with_seq_vops (gimple_stmt_iterator *, gimple_se= q); >=20=20 > /* gimple_build, functionally matching fold_buildN, outputs stmts > int the provided sequence, matching and simplifying them on-the-fly. > diff --git a/gcc/testsuite/gcc.target/aarch64/sve/acle/general/pr107920.c= b/gcc/testsuite/gcc.target/aarch64/sve/acle/general/pr107920.c > new file mode 100644 > index 00000000000..11448ed5e68 > --- /dev/null > +++ b/gcc/testsuite/gcc.target/aarch64/sve/acle/general/pr107920.c > @@ -0,0 +1,10 @@ > +/* { dg-do compile } */ > +/* { dg-options "-O1 -fno-tree-ccp -fno-tree-forwprop" } */ > + > +#include "arm_sve.h" > + > +svint8_t > +test_s8(int8_t *x) > +{ > + return svld1rq_s8 (svptrue_b8 (), &x[0]); > +}