Message ID | ZC04IGSaDzQarXvq@tucnak |
---|---|
State | New |
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 36C3B3858C20 for <patchwork@sourceware.org>; Wed, 5 Apr 2023 08:58:45 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 36C3B3858C20 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1680685125; bh=Unpzjn1naFGmXFW1ql6eic3jLX7XECtxULKeLP0l3mY=; h=Date:To:Cc:Subject:List-Id:List-Unsubscribe:List-Archive: List-Post:List-Help:List-Subscribe:From:Reply-To:From; b=Oe+vWgm6a1wnziZwKqOV62qTJ8SqdoDWgty7v0xPAfsaC2+pd2uvVzAqlOzQBLXNH Gizvg4sIxdcKGI7lsg4hAMs02VoevupZ5hKE9WBDfn/knLX0569t77bmWzIpshFMnN OskLfGSSTGVCNRaBCvL9zEn8xAanAzRr8Os9UZsI= X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by sourceware.org (Postfix) with ESMTPS id 215153858CDA for <gcc-patches@gcc.gnu.org>; Wed, 5 Apr 2023 08:58:17 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 215153858CDA Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-387-9JWj-PZsMOSWGEbx4NTjcA-1; Wed, 05 Apr 2023 04:58:13 -0400 X-MC-Unique: 9JWj-PZsMOSWGEbx4NTjcA-1 Received: from smtp.corp.redhat.com (int-mx10.intmail.prod.int.rdu2.redhat.com [10.11.54.10]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 1D79988B7A0; Wed, 5 Apr 2023 08:58:13 +0000 (UTC) Received: from tucnak.zalov.cz (unknown [10.39.192.16]) by smtp.corp.redhat.com (Postfix) with ESMTPS id CF1DD400F4F; Wed, 5 Apr 2023 08:58:12 +0000 (UTC) Received: from tucnak.zalov.cz (localhost [127.0.0.1]) by tucnak.zalov.cz (8.17.1/8.17.1) with ESMTPS id 3358wA3p3972231 (version=TLSv1.3 cipher=TLS_AES_256_GCM_SHA384 bits=256 verify=NOT); Wed, 5 Apr 2023 10:58:10 +0200 Received: (from jakub@localhost) by tucnak.zalov.cz (8.17.1/8.17.1/Submit) id 3358w9PN3972230; Wed, 5 Apr 2023 10:58:09 +0200 Date: Wed, 5 Apr 2023 10:58:08 +0200 To: Richard Biener <rguenther@suse.de>, Jeff Law <jeffreyalaw@gmail.com> Cc: gcc-patches@gcc.gnu.org Subject: [PATCH] tree-vect-generic: Fix up ICE with SSA_NAME_OCCURS_IN_ABNORMAL_PHI [PR109392] Message-ID: <ZC04IGSaDzQarXvq@tucnak> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.1 on 10.11.54.10 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=us-ascii Content-Disposition: inline X-Spam-Status: No, score=-3.3 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H2, SPF_HELO_NONE, SPF_NONE, TXREP 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: Jakub Jelinek via Gcc-patches <gcc-patches@gcc.gnu.org> Reply-To: Jakub Jelinek <jakub@redhat.com> Errors-To: gcc-patches-bounces+patchwork=sourceware.org@gcc.gnu.org Sender: "Gcc-patches" <gcc-patches-bounces+patchwork=sourceware.org@gcc.gnu.org> |
Series |
tree-vect-generic: Fix up ICE with SSA_NAME_OCCURS_IN_ABNORMAL_PHI [PR109392]
|
|
Commit Message
Jakub Jelinek
April 5, 2023, 8:58 a.m. UTC
Hi! tree_vect_extract uses gimple-fold/match.pd to attempt to simplify the BIT_FIELD_REF immediately. Unfortunately, maybe_push_res_to_seq has: /* Play safe and do not allow abnormals to be mentioned in newly created statements. */ for (unsigned int i = 0; i < num_ops; ++i) if (TREE_CODE (ops[i]) == SSA_NAME && SSA_NAME_OCCURS_IN_ABNORMAL_PHI (ops[i])) return NULL_TREE; if (num_ops > 0 && COMPARISON_CLASS_P (ops[0])) for (unsigned int i = 0; i < 2; ++i) if (TREE_CODE (TREE_OPERAND (ops[0], i)) == SSA_NAME && SSA_NAME_OCCURS_IN_ABNORMAL_PHI (TREE_OPERAND (ops[0], i))) return NULL_TREE; and so can fail in presence of SSA_NAME_OCCURS_IN_ABNORMAL_PHI SSA_NAMEs, where we then trigger the assert that maybe_push_res_to_seq doesn't return NULL. The above is perfectly reasonable when trying to actually simplify something that has been already created and let us just punt, but in the tree_vect_extract case we have to build something always. The following patch fixes it by building the BIT_FIELD_REF manually in that case if the build+simplification failed. Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? 2023-04-05 Jakub Jelinek <jakub@redhat.com> PR tree-optimization/109392 * tree-vect-generic.cc (tree_vec_extract): If maybe_push_res_to_seq fails, build BIT_FIELD_REF insn without trying to simplify it. * gcc.dg/pr109392.c: New test. Jakub
Comments
> Am 05.04.2023 um 10:58 schrieb Jakub Jelinek via Gcc-patches <gcc-patches@gcc.gnu.org>: > > Hi! > > tree_vect_extract uses gimple-fold/match.pd to attempt to simplify > the BIT_FIELD_REF immediately. > Unfortunately, maybe_push_res_to_seq has: > /* Play safe and do not allow abnormals to be mentioned in > newly created statements. */ > for (unsigned int i = 0; i < num_ops; ++i) > if (TREE_CODE (ops[i]) == SSA_NAME > && SSA_NAME_OCCURS_IN_ABNORMAL_PHI (ops[i])) > return NULL_TREE; > > if (num_ops > 0 && COMPARISON_CLASS_P (ops[0])) > for (unsigned int i = 0; i < 2; ++i) > if (TREE_CODE (TREE_OPERAND (ops[0], i)) == SSA_NAME > && SSA_NAME_OCCURS_IN_ABNORMAL_PHI (TREE_OPERAND (ops[0], i))) > return NULL_TREE; > and so can fail in presence of SSA_NAME_OCCURS_IN_ABNORMAL_PHI SSA_NAMEs, > where we then trigger the assert that maybe_push_res_to_seq doesn't return > NULL. The above is perfectly reasonable when trying to actually simplify > something that has been already created and let us just punt, but in the > tree_vect_extract case we have to build something always. > > The following patch fixes it by building the BIT_FIELD_REF manually in > that case if the build+simplification failed. > > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? Ok, but maybe there’s a gimple_build overload that meanwhile implements the desired semantics? It would probably need to specify the valueization hook to follow SSA edges beyond the sequence we’re currently building. Richard > 2023-04-05 Jakub Jelinek <jakub@redhat.com> > > PR tree-optimization/109392 > * tree-vect-generic.cc (tree_vec_extract): If maybe_push_res_to_seq > fails, build BIT_FIELD_REF insn without trying to simplify it. > > * gcc.dg/pr109392.c: New test. > > --- gcc/tree-vect-generic.cc.jj 2023-03-23 10:02:18.997935620 +0100 > +++ gcc/tree-vect-generic.cc 2023-04-04 14:28:32.977729134 +0200 > @@ -174,7 +174,16 @@ tree_vec_extract (gimple_stmt_iterator * > opr.resimplify (NULL, follow_all_ssa_edges); > gimple_seq stmts = NULL; > tree res = maybe_push_res_to_seq (&opr, &stmts); > - gcc_assert (res); > + if (!res) > + { > + /* This can happen if SSA_NAME_OCCURS_IN_ABNORMAL_PHI are > + used. Build BIT_FIELD_REF manually otherwise. */ > + t = build3 (BIT_FIELD_REF, type, t, bitsize, bitpos); > + res = make_ssa_name (type); > + gimple *g = gimple_build_assign (res, t); > + gsi_insert_before (gsi, g, GSI_SAME_STMT); > + return res; > + } > gsi_insert_seq_before (gsi, stmts, GSI_SAME_STMT); > return res; > } > --- gcc/testsuite/gcc.dg/pr109392.c.jj 2023-04-04 14:36:03.096109943 +0200 > +++ gcc/testsuite/gcc.dg/pr109392.c 2023-04-04 14:35:39.784452751 +0200 > @@ -0,0 +1,15 @@ > +/* PR tree-optimization/109392 */ > +/* { dg-do compile } */ > +/* { dg-options "-O2 -Wno-psabi" } */ > + > +typedef short __attribute__ ((__vector_size__ (64))) V; > +V v, w; > +void bar (void) __attribute__((returns_twice)); > + > +V > +foo (V a, V b) > +{ > + bar (); > + b &= v < b; > + return (V) { foo (b, w)[3], (V) {}[3] }; > +} > > Jakub >
On Wed, Apr 05, 2023 at 02:11:08PM +0200, Richard Biener wrote: > Ok, but maybe there’s a gimple_build overload that meanwhile implements > the desired semantics? It would probably need to specify the valueization > hook to follow SSA edges beyond the sequence we’re currently building. Jeff has apparently committed the patch in the meantime. For gimple_build, did you mean to use it just for this fallback case, or instead of the initial resimplification as well? > > 2023-04-05 Jakub Jelinek <jakub@redhat.com> > > > > PR tree-optimization/109392 > > * tree-vect-generic.cc (tree_vec_extract): If maybe_push_res_to_seq > > fails, build BIT_FIELD_REF insn without trying to simplify it. > > > > * gcc.dg/pr109392.c: New test. > > > > --- gcc/tree-vect-generic.cc.jj 2023-03-23 10:02:18.997935620 +0100 > > +++ gcc/tree-vect-generic.cc 2023-04-04 14:28:32.977729134 +0200 > > @@ -174,7 +174,16 @@ tree_vec_extract (gimple_stmt_iterator * > > opr.resimplify (NULL, follow_all_ssa_edges); > > gimple_seq stmts = NULL; > > tree res = maybe_push_res_to_seq (&opr, &stmts); > > - gcc_assert (res); > > + if (!res) > > + { > > + /* This can happen if SSA_NAME_OCCURS_IN_ABNORMAL_PHI are > > + used. Build BIT_FIELD_REF manually otherwise. */ > > + t = build3 (BIT_FIELD_REF, type, t, bitsize, bitpos); > > + res = make_ssa_name (type); > > + gimple *g = gimple_build_assign (res, t); > > + gsi_insert_before (gsi, g, GSI_SAME_STMT); > > + return res; > > + } > > gsi_insert_seq_before (gsi, stmts, GSI_SAME_STMT); > > return res; > > } Jakub
On Tue, 11 Apr 2023, Jakub Jelinek wrote: > On Wed, Apr 05, 2023 at 02:11:08PM +0200, Richard Biener wrote: > > Ok, but maybe there?s a gimple_build overload that meanwhile implements > > the desired semantics? It would probably need to specify the valueization > > hook to follow SSA edges beyond the sequence we?re currently building. > > Jeff has apparently committed the patch in the meantime. > For gimple_build, did you mean to use it just for this fallback case, > or instead of the initial resimplification as well? I was originally thinking of the initial resimplification. But yes, using gimple_build would have simplified it to if (!res) res = gimple_build (&stmts, BIT_FIELD_REF, type, t, bitsize, bitpos); With the gsi overloads the whole function should now be able to be turned into return gimple_build (gsi, true, GSI_SAME_STMT, UNKNOWN_LOCATION, BIT_FIELD_REF, type, t, bitsize, bitpos); I think. Note that's not 100% equivalent to what we do now, so it's probably better to defer to stage1. What it doesn't do is /* We're using the resimplify API and maybe_push_res_to_seq to simplify the BIT_FIELD_REF but restrict the simplification to a single stmt while at the same time following SSA edges for simplification with already emitted CTORs. */ which is achieved by passing NULL as the gimple_seq * argument to .resimplify. The above replacement would instead do what a later forwprop pass would do when folding. ISTR I wanted to do minimal changes when rewriting this. Richard. > > > 2023-04-05 Jakub Jelinek <jakub@redhat.com> > > > > > > PR tree-optimization/109392 > > > * tree-vect-generic.cc (tree_vec_extract): If maybe_push_res_to_seq > > > fails, build BIT_FIELD_REF insn without trying to simplify it. > > > > > > * gcc.dg/pr109392.c: New test. > > > > > > --- gcc/tree-vect-generic.cc.jj 2023-03-23 10:02:18.997935620 +0100 > > > +++ gcc/tree-vect-generic.cc 2023-04-04 14:28:32.977729134 +0200 > > > @@ -174,7 +174,16 @@ tree_vec_extract (gimple_stmt_iterator * > > > opr.resimplify (NULL, follow_all_ssa_edges); > > > gimple_seq stmts = NULL; > > > tree res = maybe_push_res_to_seq (&opr, &stmts); > > > - gcc_assert (res); > > > + if (!res) > > > + { > > > + /* This can happen if SSA_NAME_OCCURS_IN_ABNORMAL_PHI are > > > + used. Build BIT_FIELD_REF manually otherwise. */ > > > + t = build3 (BIT_FIELD_REF, type, t, bitsize, bitpos); > > > + res = make_ssa_name (type); > > > + gimple *g = gimple_build_assign (res, t); > > > + gsi_insert_before (gsi, g, GSI_SAME_STMT); > > > + return res; > > > + } > > > gsi_insert_seq_before (gsi, stmts, GSI_SAME_STMT); > > > return res; > > > } > > Jakub > >
--- gcc/tree-vect-generic.cc.jj 2023-03-23 10:02:18.997935620 +0100 +++ gcc/tree-vect-generic.cc 2023-04-04 14:28:32.977729134 +0200 @@ -174,7 +174,16 @@ tree_vec_extract (gimple_stmt_iterator * opr.resimplify (NULL, follow_all_ssa_edges); gimple_seq stmts = NULL; tree res = maybe_push_res_to_seq (&opr, &stmts); - gcc_assert (res); + if (!res) + { + /* This can happen if SSA_NAME_OCCURS_IN_ABNORMAL_PHI are + used. Build BIT_FIELD_REF manually otherwise. */ + t = build3 (BIT_FIELD_REF, type, t, bitsize, bitpos); + res = make_ssa_name (type); + gimple *g = gimple_build_assign (res, t); + gsi_insert_before (gsi, g, GSI_SAME_STMT); + return res; + } gsi_insert_seq_before (gsi, stmts, GSI_SAME_STMT); return res; } --- gcc/testsuite/gcc.dg/pr109392.c.jj 2023-04-04 14:36:03.096109943 +0200 +++ gcc/testsuite/gcc.dg/pr109392.c 2023-04-04 14:35:39.784452751 +0200 @@ -0,0 +1,15 @@ +/* PR tree-optimization/109392 */ +/* { dg-do compile } */ +/* { dg-options "-O2 -Wno-psabi" } */ + +typedef short __attribute__ ((__vector_size__ (64))) V; +V v, w; +void bar (void) __attribute__((returns_twice)); + +V +foo (V a, V b) +{ + bar (); + b &= v < b; + return (V) { foo (b, w)[3], (V) {}[3] }; +}