[aarch64] PR107920 - Fix incorrect handling of virtual operands in svld1rq_impl::fold
Message ID | CAAgBjMnaLY=sigq_+fXpBZ++UpEw4AD_XdNL4H-1Gy4Knp+cAw@mail.gmail.com |
---|---|
State | Committed |
Headers |
Return-Path: <gcc-patches-bounces+patchwork=sourceware.org@gcc.gnu.org> X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 742513855150 for <patchwork@sourceware.org>; Fri, 2 Dec 2022 07:22:44 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 742513855150 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1669965764; bh=qUTLqkdODCQqZRjnUH4hANppAgmux57gU7R6+VgTnfM=; h=Date:Subject:To:List-Id:List-Unsubscribe:List-Archive:List-Post: List-Help:List-Subscribe:From:Reply-To:From; b=Ichx/IxqAG64q6nddKjyzA55R4hy4k5+2Tksvcrq6XAHP0Jx7trb92XedQgLjF0Wj IbVBoEZvRMeQZ1vEUQDl0buGJ64hl7P1xVgARZ9zkxFiHCwx4qmc1vfReHnARQGTny 1ALqYMFvcqqKI70aZTUZzuQtgLOmzjq6KWujR/Yc= X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from mail-lj1-x233.google.com (mail-lj1-x233.google.com [IPv6:2a00:1450:4864:20::233]) by sourceware.org (Postfix) with ESMTPS id A43E13858D32 for <gcc-patches@gcc.gnu.org>; Fri, 2 Dec 2022 07:22:16 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org A43E13858D32 Received: by mail-lj1-x233.google.com with SMTP id a7so4445682ljq.12 for <gcc-patches@gcc.gnu.org>; Thu, 01 Dec 2022 23:22:16 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=to:subject:message-id:date:from:mime-version:x-gm-message-state :from:to:cc:subject:date:message-id:reply-to; bh=qUTLqkdODCQqZRjnUH4hANppAgmux57gU7R6+VgTnfM=; b=MU7yTVd/3TLMxzQXgUFZ95K7bRUuRspl9gUFHRTzZW2LMMWhSI8ywEMyARGHw+v9Gq ucNbDbyykZJKzncrDEnm9yDvzQ74LFXzWA6Cq0DhEtrIFLLl5HrkEZEZHQlobS6HonS+ g4Dlyle2AbzCfsclYjbc7GgjsdRp4zHiUlnWhuSDtWZ3SQi/FhCgbRqSiH59FojGpV4m t3DirRQRSKFgQ+9uiVotfJx12EmKrlvhkWrUQ/6fezIPi5OZZeLSssh54b0yJEqG2tsk fr896m8zjx96Y/bUj+vRdL/MaYPKwyVk2DqIQQB0ityq7i39LMvY8LGIqIYxxBAz36W/ og+w== X-Gm-Message-State: ANoB5pmfCeucgk5muDMyu3d4dnEXNDyXuZs76VF6NafPwxhL2myA3IbN vaX9S0sa8OwbEROze2i3/WthZT6uDDJj5INdXxD6QQLU+51WSuAA X-Google-Smtp-Source: AA0mqf6T7tHfknfPkZ1/St9/N3XSg/XyQeuP/csarNvRc22dtTQFNDRO8Isk8seRyq+zhwmD/FuWvG4NN/cwnIqInJE= X-Received: by 2002:a2e:8ed8:0:b0:279:78e1:c7a4 with SMTP id e24-20020a2e8ed8000000b0027978e1c7a4mr14168355ljl.529.1669965734362; Thu, 01 Dec 2022 23:22:14 -0800 (PST) MIME-Version: 1.0 Date: Fri, 2 Dec 2022 12:51:37 +0530 Message-ID: <CAAgBjMnaLY=sigq_+fXpBZ++UpEw4AD_XdNL4H-1Gy4Knp+cAw@mail.gmail.com> Subject: [aarch64] PR107920 - Fix incorrect handling of virtual operands in svld1rq_impl::fold To: gcc Patches <gcc-patches@gcc.gnu.org>, Richard Sandiford <richard.sandiford@arm.com> Content-Type: multipart/mixed; boundary="00000000000062a5d205eed33206" X-Spam-Status: No, score=-8.5 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, KAM_SHORT, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, 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 X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list <gcc-patches.gcc.gnu.org> List-Unsubscribe: <https://gcc.gnu.org/mailman/options/gcc-patches>, <mailto:gcc-patches-request@gcc.gnu.org?subject=unsubscribe> List-Archive: <https://gcc.gnu.org/pipermail/gcc-patches/> List-Post: <mailto:gcc-patches@gcc.gnu.org> List-Help: <mailto:gcc-patches-request@gcc.gnu.org?subject=help> List-Subscribe: <https://gcc.gnu.org/mailman/listinfo/gcc-patches>, <mailto:gcc-patches-request@gcc.gnu.org?subject=subscribe> From: Prathamesh Kulkarni via Gcc-patches <gcc-patches@gcc.gnu.org> Reply-To: Prathamesh Kulkarni <prathamesh.kulkarni@linaro.org> Errors-To: gcc-patches-bounces+patchwork=sourceware.org@gcc.gnu.org Sender: "Gcc-patches" <gcc-patches-bounces+patchwork=sourceware.org@gcc.gnu.org> |
Series |
[aarch64] PR107920 - Fix incorrect handling of virtual operands in svld1rq_impl::fold
|
|
Commit Message
Prathamesh Kulkarni
Dec. 2, 2022, 7:21 a.m. UTC
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=armv8.2-a+sve -O1 -fno-tree-ccp -fno-tree-forwprop: during GIMPLE pass: fre pr107920.c: In function ‘test_s8’: pr107920.c:7:1: internal compiler error: in execute_todo, at passes.cc:2140 7 | } | ^ 0x7b03d0 execute_todo ../../gcc/gcc/passes.cc:2140 because of incorrect handling of virtual operands in svld1rq_impl::fold: # VUSE <.MEM> _5 = MEM <vector(16) signed char> [(signed char * {ref-all})x_3(D)]; _4 = 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: <bb 2> : # VUSE <.MEM_2(D)> _5 = MEM <vector(16) signed char> [(signed char * {ref-all})x_3(D)]; _4 = 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 ? Thanks, Prathamesh
Comments
Prathamesh Kulkarni <prathamesh.kulkarni@linaro.org> 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=armv8.2-a+sve -O1 -fno-tree-ccp -fno-tree-forwprop: > during GIMPLE pass: fre > pr107920.c: In function ‘test_s8’: > pr107920.c:7:1: internal compiler error: in execute_todo, at passes.cc:2140 > 7 | } > | ^ > 0x7b03d0 execute_todo > ../../gcc/gcc/passes.cc:2140 > > because of incorrect handling of virtual operands in svld1rq_impl::fold: > # VUSE <.MEM> > _5 = MEM <vector(16) signed char> [(signed char * {ref-all})x_3(D)]; > _4 = 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: > <bb 2> : > # VUSE <.MEM_2(D)> > _5 = MEM <vector(16) signed char> [(signed char * {ref-all})x_3(D)]; > _4 = 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" > > using namespace aarch64_sve; > > @@ -1232,7 +1233,9 @@ public: > tree mem_ref_op = fold_build2 (MEM_REF, access_type, arg1, zero); > gimple *mem_ref_stmt > = gimple_build_assign (mem_ref_lhs, mem_ref_op); > - gsi_insert_before (f.gsi, mem_ref_stmt, GSI_SAME_STMT); > + > + gimple_seq stmts = NULL; > + gimple_seq_add_stmt_without_update (&stmts, mem_ref_stmt); > > int source_nelts = 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 = build_vector_type (ssizetype, lhs_len); > tree mask = 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 = 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; > } > > 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. */ > > -static void > +void > gsi_replace_with_seq_vops (gimple_stmt_iterator *si_p, gimple_seq stmts) > { > gimple *stmt = 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 = 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_seq); > > /* 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]); > +}
On Tue, 6 Dec 2022 at 00:08, Richard Sandiford <richard.sandiford@arm.com> wrote: > > Prathamesh Kulkarni <prathamesh.kulkarni@linaro.org> 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=armv8.2-a+sve -O1 -fno-tree-ccp -fno-tree-forwprop: > > during GIMPLE pass: fre > > pr107920.c: In function ‘test_s8’: > > pr107920.c:7:1: internal compiler error: in execute_todo, at passes.cc:2140 > > 7 | } > > | ^ > > 0x7b03d0 execute_todo > > ../../gcc/gcc/passes.cc:2140 > > > > because of incorrect handling of virtual operands in svld1rq_impl::fold: > > # VUSE <.MEM> > > _5 = MEM <vector(16) signed char> [(signed char * {ref-all})x_3(D)]; > > _4 = 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: > > <bb 2> : > > # VUSE <.MEM_2(D)> > > _5 = MEM <vector(16) signed char> [(signed char * {ref-all})x_3(D)]; > > _4 = 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 for the suggestions. Is the attached patch OK to commit after bootstrap+test ? Thanks, Prathamesh > > 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" > > > > using namespace aarch64_sve; > > > > @@ -1232,7 +1233,9 @@ public: > > tree mem_ref_op = fold_build2 (MEM_REF, access_type, arg1, zero); > > gimple *mem_ref_stmt > > = gimple_build_assign (mem_ref_lhs, mem_ref_op); > > - gsi_insert_before (f.gsi, mem_ref_stmt, GSI_SAME_STMT); > > + > > + gimple_seq stmts = NULL; > > + gimple_seq_add_stmt_without_update (&stmts, mem_ref_stmt); > > > > int source_nelts = 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 = build_vector_type (ssizetype, lhs_len); > > tree mask = 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 = 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; > > } > > > > 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. */ > > > > -static void > > +void > > gsi_replace_with_seq_vops (gimple_stmt_iterator *si_p, gimple_seq stmts) > > { > > gimple *stmt = 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 = 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_seq); > > > > /* 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]); > > +} gcc/ChangeLog: PR target/107920 * config/aarch64/aarch64-sve-builtins-base.cc: Use gsi_replace_with_seq_vops to handle virtual operands, and gate the transform on !flag_non_call_exceptions. * gimple-fold.cc (gsi_replace_with_seq_vops): Make function non static. * gimple-fold.h (gsi_replace_with_seq_vops): Declare. gcc/testsuite/ChangeLog: PR target/107920 * gcc.target/aarch64/sve/acle/general/pr107920.c: New test. diff --git a/gcc/config/aarch64/aarch64-sve-builtins-base.cc b/gcc/config/aarch64/aarch64-sve-builtins-base.cc index 6347407555f..d52ec083ed0 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" using namespace aarch64_sve; @@ -1209,7 +1210,8 @@ public: vectype is the corresponding ADVSIMD type. */ if (!BYTES_BIG_ENDIAN - && integer_all_onesp (arg0)) + && integer_all_onesp (arg0) + && !flag_non_call_exceptions) { tree lhs = gimple_call_lhs (f.call); tree lhs_type = TREE_TYPE (lhs); @@ -1232,7 +1234,9 @@ public: tree mem_ref_op = fold_build2 (MEM_REF, access_type, arg1, zero); gimple *mem_ref_stmt = gimple_build_assign (mem_ref_lhs, mem_ref_op); - gsi_insert_before (f.gsi, mem_ref_stmt, GSI_SAME_STMT); + + gimple_seq stmts = NULL; + gimple_seq_add_stmt_without_update (&stmts, mem_ref_stmt); int source_nelts = TYPE_VECTOR_SUBPARTS (access_type).to_constant (); vec_perm_builder sel (lhs_len, source_nelts, 1); @@ -1245,8 +1249,11 @@ public: indices)); tree mask_type = build_vector_type (ssizetype, lhs_len); tree mask = 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 = 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; } 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. */ -static void +void gsi_replace_with_seq_vops (gimple_stmt_iterator *si_p, gimple_seq stmts) { gimple *stmt = 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 = 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_seq); /* 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]); +}
Prathamesh Kulkarni <prathamesh.kulkarni@linaro.org> writes: > On Tue, 6 Dec 2022 at 00:08, Richard Sandiford > <richard.sandiford@arm.com> wrote: >> >> Prathamesh Kulkarni <prathamesh.kulkarni@linaro.org> 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=armv8.2-a+sve -O1 -fno-tree-ccp -fno-tree-forwprop: >> > during GIMPLE pass: fre >> > pr107920.c: In function ‘test_s8’: >> > pr107920.c:7:1: internal compiler error: in execute_todo, at passes.cc:2140 >> > 7 | } >> > | ^ >> > 0x7b03d0 execute_todo >> > ../../gcc/gcc/passes.cc:2140 >> > >> > because of incorrect handling of virtual operands in svld1rq_impl::fold: >> > # VUSE <.MEM> >> > _5 = MEM <vector(16) signed char> [(signed char * {ref-all})x_3(D)]; >> > _4 = 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: >> > <bb 2> : >> > # VUSE <.MEM_2(D)> >> > _5 = MEM <vector(16) signed char> [(signed char * {ref-all})x_3(D)]; >> > _4 = 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 for the suggestions. Is the attached patch OK to commit after > bootstrap+test ? Please add the test above to g++.target/aarch64/sve, with a scan-assembler for __cxa_begin_catch. OK with that change, thanks. Richard > > Thanks, > Prathamesh >> >> 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" >> > >> > using namespace aarch64_sve; >> > >> > @@ -1232,7 +1233,9 @@ public: >> > tree mem_ref_op = fold_build2 (MEM_REF, access_type, arg1, zero); >> > gimple *mem_ref_stmt >> > = gimple_build_assign (mem_ref_lhs, mem_ref_op); >> > - gsi_insert_before (f.gsi, mem_ref_stmt, GSI_SAME_STMT); >> > + >> > + gimple_seq stmts = NULL; >> > + gimple_seq_add_stmt_without_update (&stmts, mem_ref_stmt); >> > >> > int source_nelts = 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 = build_vector_type (ssizetype, lhs_len); >> > tree mask = 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 = 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; >> > } >> > >> > 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. */ >> > >> > -static void >> > +void >> > gsi_replace_with_seq_vops (gimple_stmt_iterator *si_p, gimple_seq stmts) >> > { >> > gimple *stmt = 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 = 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_seq); >> > >> > /* 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]); >> > +} > > gcc/ChangeLog: > PR target/107920 > * config/aarch64/aarch64-sve-builtins-base.cc: Use > gsi_replace_with_seq_vops to handle virtual operands, and gate > the transform on !flag_non_call_exceptions. > * gimple-fold.cc (gsi_replace_with_seq_vops): Make function non static. > * gimple-fold.h (gsi_replace_with_seq_vops): Declare. > > gcc/testsuite/ChangeLog: > PR target/107920 > * gcc.target/aarch64/sve/acle/general/pr107920.c: New test. > > diff --git a/gcc/config/aarch64/aarch64-sve-builtins-base.cc b/gcc/config/aarch64/aarch64-sve-builtins-base.cc > index 6347407555f..d52ec083ed0 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" > > using namespace aarch64_sve; > > @@ -1209,7 +1210,8 @@ public: > vectype is the corresponding ADVSIMD type. */ > > if (!BYTES_BIG_ENDIAN > - && integer_all_onesp (arg0)) > + && integer_all_onesp (arg0) > + && !flag_non_call_exceptions) > { > tree lhs = gimple_call_lhs (f.call); > tree lhs_type = TREE_TYPE (lhs); > @@ -1232,7 +1234,9 @@ public: > tree mem_ref_op = fold_build2 (MEM_REF, access_type, arg1, zero); > gimple *mem_ref_stmt > = gimple_build_assign (mem_ref_lhs, mem_ref_op); > - gsi_insert_before (f.gsi, mem_ref_stmt, GSI_SAME_STMT); > + > + gimple_seq stmts = NULL; > + gimple_seq_add_stmt_without_update (&stmts, mem_ref_stmt); > > int source_nelts = TYPE_VECTOR_SUBPARTS (access_type).to_constant (); > vec_perm_builder sel (lhs_len, source_nelts, 1); > @@ -1245,8 +1249,11 @@ public: > indices)); > tree mask_type = build_vector_type (ssizetype, lhs_len); > tree mask = 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 = 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; > } > > 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. */ > > -static void > +void > gsi_replace_with_seq_vops (gimple_stmt_iterator *si_p, gimple_seq stmts) > { > gimple *stmt = 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 = 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_seq); > > /* 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]); > +}
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" using namespace aarch64_sve; @@ -1232,7 +1233,9 @@ public: tree mem_ref_op = fold_build2 (MEM_REF, access_type, arg1, zero); gimple *mem_ref_stmt = gimple_build_assign (mem_ref_lhs, mem_ref_op); - gsi_insert_before (f.gsi, mem_ref_stmt, GSI_SAME_STMT); + + gimple_seq stmts = NULL; + gimple_seq_add_stmt_without_update (&stmts, mem_ref_stmt); int source_nelts = 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 = build_vector_type (ssizetype, lhs_len); tree mask = 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 = 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; } 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. */ -static void +void gsi_replace_with_seq_vops (gimple_stmt_iterator *si_p, gimple_seq stmts) { gimple *stmt = 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 = 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_seq); /* 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]); +}