Message ID | 20220413074636.9269E13A91@imap2.suse-dmz.suse.de |
---|---|
State | Committed |
Commit | 4e892de6774f86540d36385701aa7b0a2bba5155 |
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 A31543857341 for <patchwork@sourceware.org>; Wed, 13 Apr 2022 07:47:06 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org A31543857341 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1649836026; bh=QdqJyM/u0ab2E2crddi3TE1+xSz/3fOp5acANW3Jejo=; h=Date:To:Subject:List-Id:List-Unsubscribe:List-Archive:List-Post: List-Help:List-Subscribe:From:Reply-To:From; b=mnWp95B4KGn7UXvA7I4O6rFMjjRWMWgayty3dDRptkCQFwvzUXGEXQF4xEnZpUUGW jDb/m8gBzGO4YaCqJypAlkpiuyN847tYoKMgiXc9zqBS2pQ6HO32GoGSpGuRfo/4+O XbVItJa9BE1PBCQq8JRxvWDKCzcNKg/p9YYBN95k= X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from smtp-out1.suse.de (smtp-out1.suse.de [195.135.220.28]) by sourceware.org (Postfix) with ESMTPS id 8DBFF3858C27 for <gcc-patches@gcc.gnu.org>; Wed, 13 Apr 2022 07:46:37 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 8DBFF3858C27 Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by smtp-out1.suse.de (Postfix) with ESMTPS id A41C4210EA for <gcc-patches@gcc.gnu.org>; Wed, 13 Apr 2022 07:46:36 +0000 (UTC) Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by imap2.suse-dmz.suse.de (Postfix) with ESMTPS id 9269E13A91 for <gcc-patches@gcc.gnu.org>; Wed, 13 Apr 2022 07:46:36 +0000 (UTC) Received: from dovecot-director2.suse.de ([192.168.254.65]) by imap2.suse-dmz.suse.de with ESMTPSA id Lbl9Itx/VmJnbQAAMHmgww (envelope-from <rguenther@suse.de>) for <gcc-patches@gcc.gnu.org>; Wed, 13 Apr 2022 07:46:36 +0000 Date: Wed, 13 Apr 2022 09:46:36 +0200 (CEST) To: gcc-patches@gcc.gnu.org Subject: [PATCH] tree-optimization/105250 - adjust fold_convertible_p PR105140 fix MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Message-Id: <20220413074636.9269E13A91@imap2.suse-dmz.suse.de> X-Spam-Status: No, score=-11.2 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, SCC_5_SHORT_WORD_LINES, SPF_HELO_NONE, SPF_PASS, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) 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: Richard Biener via Gcc-patches <gcc-patches@gcc.gnu.org> Reply-To: Richard Biener <rguenther@suse.de> 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-optimization/105250 - adjust fold_convertible_p PR105140 fix
|
|
Commit Message
Richard Biener
April 13, 2022, 7:46 a.m. UTC
The following reverts the original PR105140 fix and goes for instead applying the additional fold_convert constraint for VECTOR_TYPE conversions also to fold_convertible_p. I did not try sanitizing all of this at this point. Bootstrapped on x86_64-unknown-linux-gnu, testing in progress. 2022-04-13 Richard Biener <rguenther@suse.de> PR tree-optimization/105250 * fold-const.cc (fold_convertible_p): Revert r12-7979-geaaf77dd85c333, instead check for size equality of the vector types involved. * gcc.dg/pr105250.c: New testcase. --- gcc/fold-const.cc | 7 +++---- gcc/testsuite/gcc.dg/pr105250.c | 29 +++++++++++++++++++++++++++++ 2 files changed, 32 insertions(+), 4 deletions(-) create mode 100644 gcc/testsuite/gcc.dg/pr105250.c
Comments
Richard Biener via Gcc-patches <gcc-patches@gcc.gnu.org> writes: > The following reverts the original PR105140 fix and goes for instead > applying the additional fold_convert constraint for VECTOR_TYPE > conversions also to fold_convertible_p. I did not try sanitizing > all of this at this point. > > Bootstrapped on x86_64-unknown-linux-gnu, testing in progress. > > 2022-04-13 Richard Biener <rguenther@suse.de> > > PR tree-optimization/105250 > * fold-const.cc (fold_convertible_p): Revert > r12-7979-geaaf77dd85c333, instead check for size equality > of the vector types involved. This doesn't look right, and I think it'll break SVE. For one thing, the tree_int_cst_equal check is bound to fail for variable-length vectors. But also, the idea was to allow element-wise conversions between different vector sizes. For example, you can do a nop/convert from V4SI to V4DI, which converts 4 SIs to 4 DIs. This is used a lot for conversions to and from “partial” SVE vectors, where smaller elements are stored in wider containers. Thanks, Richard > > * gcc.dg/pr105250.c: New testcase. > --- > gcc/fold-const.cc | 7 +++---- > gcc/testsuite/gcc.dg/pr105250.c | 29 +++++++++++++++++++++++++++++ > 2 files changed, 32 insertions(+), 4 deletions(-) > create mode 100644 gcc/testsuite/gcc.dg/pr105250.c > > diff --git a/gcc/fold-const.cc b/gcc/fold-const.cc > index 7226bc5af01..a57ad0739fb 100644 > --- a/gcc/fold-const.cc > +++ b/gcc/fold-const.cc > @@ -2379,13 +2379,12 @@ build_zero_vector (tree type) > return build_vector_from_val (type, t); > } > > -/* Returns true, if ARG, an operand or a type, is convertible to TYPE > - using a NOP_EXPR. */ > +/* Returns true, if ARG is convertible to TYPE using a NOP_EXPR. */ > > bool > fold_convertible_p (const_tree type, const_tree arg) > { > - const_tree orig = TYPE_P (arg) ? arg : TREE_TYPE (arg); > + const_tree orig = TREE_TYPE (arg); > > if (type == orig) > return true; > @@ -2417,7 +2416,7 @@ fold_convertible_p (const_tree type, const_tree arg) > return (VECTOR_TYPE_P (orig) > && known_eq (TYPE_VECTOR_SUBPARTS (type), > TYPE_VECTOR_SUBPARTS (orig)) > - && fold_convertible_p (TREE_TYPE (type), TREE_TYPE (orig))); > + && tree_int_cst_equal (TYPE_SIZE (type), TYPE_SIZE (orig))); > > default: > return false; > diff --git a/gcc/testsuite/gcc.dg/pr105250.c b/gcc/testsuite/gcc.dg/pr105250.c > new file mode 100644 > index 00000000000..665dd95d8cb > --- /dev/null > +++ b/gcc/testsuite/gcc.dg/pr105250.c > @@ -0,0 +1,29 @@ > +/* { dg-do compile } */ > +/* { dg-options "-w -Wno-psabi -O2" } */ > + > +typedef int __attribute__((__vector_size__(4))) T; > +typedef int __attribute__((__vector_size__(8))) U; > +typedef int __attribute__((__vector_size__(16))) V; > +typedef int __attribute__((__vector_size__(32))) W; > +typedef _Float32 __attribute__((__vector_size__(16))) F; > +typedef _Float64 __attribute__((__vector_size__(32))) G; > +void foo(); > + > +foo(int, int, int, int, U, U, V, V, W, W, int, > + T, int, U, U, V, V, W, W, T, > + T, int, U, U, V, V, W, W, T, > + T, int, W, W, T, T, int, int, int, > + int, int, int, W, int, int, int, int, int, int, > + V, W, T, int, int, U, F, int, int, int, > + int, int, int, G) > +{ > + foo(0, 0, 0, 0, (U){}, (U){}, (V){}, (V){}, (W){}, > + (W){}, 2, (T){}, 0, 0, 0, 0, (U){}, (U){}, > + (V){}, (V){}, (W){}, (W){}, (T){}, > + (T){}, 0, 0, 0, 0, (U){}, (U){}, (V){}, > + (V){}, (W){}, (W){}, (T){}, (T){}, 0, 0, 0, > + 0, 0, 0, (T){}, > + (T){}, (W){}, > + (W){}, (T){}, (T){}, 0, 0, 0, 0, 0, 0, (W){}, > + (V){}, (W){}, (T){}, 0, 0, (U){}, (F){}); > +}
On Wed, 13 Apr 2022, Richard Sandiford wrote: > Richard Biener via Gcc-patches <gcc-patches@gcc.gnu.org> writes: > > The following reverts the original PR105140 fix and goes for instead > > applying the additional fold_convert constraint for VECTOR_TYPE > > conversions also to fold_convertible_p. I did not try sanitizing > > all of this at this point. > > > > Bootstrapped on x86_64-unknown-linux-gnu, testing in progress. > > > > 2022-04-13 Richard Biener <rguenther@suse.de> > > > > PR tree-optimization/105250 > > * fold-const.cc (fold_convertible_p): Revert > > r12-7979-geaaf77dd85c333, instead check for size equality > > of the vector types involved. > > This doesn't look right, and I think it'll break SVE. For one > thing, the tree_int_cst_equal check is bound to fail for > variable-length vectors. > > But also, the idea was to allow element-wise conversions between > different vector sizes. For example, you can do a nop/convert > from V4SI to V4DI, which converts 4 SIs to 4 DIs. This is used > a lot for conversions to and from “partial” SVE vectors, where smaller > elements are stored in wider containers. But fold_convertible_p is used as guard for fold_convert in a lot of places and that will simply ICE when there's a mismatch in size as can be seen in the testcase. Note the code as before the previous fix couldn't really have worked as expected. Is there any testcase that will "break" now? I realize the fold_convertible_p comment says "using a NOP_EXPR" which means it might conver a narrower set of conversions than fold_convert (which will happily use FLOAT_EXPR and friends), but still it should allow fold_convert to build the conversion. The alternative would have been to emit a NOP_EXPR from fold_convert for vector type conversions (with the correct constraints), but then not all targets support those, so we'd need a target support check in fold_convertible_p then? Richard. > Thanks, > Richard > > > > > * gcc.dg/pr105250.c: New testcase. > > --- > > gcc/fold-const.cc | 7 +++---- > > gcc/testsuite/gcc.dg/pr105250.c | 29 +++++++++++++++++++++++++++++ > > 2 files changed, 32 insertions(+), 4 deletions(-) > > create mode 100644 gcc/testsuite/gcc.dg/pr105250.c > > > > diff --git a/gcc/fold-const.cc b/gcc/fold-const.cc > > index 7226bc5af01..a57ad0739fb 100644 > > --- a/gcc/fold-const.cc > > +++ b/gcc/fold-const.cc > > @@ -2379,13 +2379,12 @@ build_zero_vector (tree type) > > return build_vector_from_val (type, t); > > } > > > > -/* Returns true, if ARG, an operand or a type, is convertible to TYPE > > - using a NOP_EXPR. */ > > +/* Returns true, if ARG is convertible to TYPE using a NOP_EXPR. */ > > > > bool > > fold_convertible_p (const_tree type, const_tree arg) > > { > > - const_tree orig = TYPE_P (arg) ? arg : TREE_TYPE (arg); > > + const_tree orig = TREE_TYPE (arg); > > > > if (type == orig) > > return true; > > @@ -2417,7 +2416,7 @@ fold_convertible_p (const_tree type, const_tree arg) > > return (VECTOR_TYPE_P (orig) > > && known_eq (TYPE_VECTOR_SUBPARTS (type), > > TYPE_VECTOR_SUBPARTS (orig)) > > - && fold_convertible_p (TREE_TYPE (type), TREE_TYPE (orig))); > > + && tree_int_cst_equal (TYPE_SIZE (type), TYPE_SIZE (orig))); > > > > default: > > return false; > > diff --git a/gcc/testsuite/gcc.dg/pr105250.c b/gcc/testsuite/gcc.dg/pr105250.c > > new file mode 100644 > > index 00000000000..665dd95d8cb > > --- /dev/null > > +++ b/gcc/testsuite/gcc.dg/pr105250.c > > @@ -0,0 +1,29 @@ > > +/* { dg-do compile } */ > > +/* { dg-options "-w -Wno-psabi -O2" } */ > > + > > +typedef int __attribute__((__vector_size__(4))) T; > > +typedef int __attribute__((__vector_size__(8))) U; > > +typedef int __attribute__((__vector_size__(16))) V; > > +typedef int __attribute__((__vector_size__(32))) W; > > +typedef _Float32 __attribute__((__vector_size__(16))) F; > > +typedef _Float64 __attribute__((__vector_size__(32))) G; > > +void foo(); > > + > > +foo(int, int, int, int, U, U, V, V, W, W, int, > > + T, int, U, U, V, V, W, W, T, > > + T, int, U, U, V, V, W, W, T, > > + T, int, W, W, T, T, int, int, int, > > + int, int, int, W, int, int, int, int, int, int, > > + V, W, T, int, int, U, F, int, int, int, > > + int, int, int, G) > > +{ > > + foo(0, 0, 0, 0, (U){}, (U){}, (V){}, (V){}, (W){}, > > + (W){}, 2, (T){}, 0, 0, 0, 0, (U){}, (U){}, > > + (V){}, (V){}, (W){}, (W){}, (T){}, > > + (T){}, 0, 0, 0, 0, (U){}, (U){}, (V){}, > > + (V){}, (W){}, (W){}, (T){}, (T){}, 0, 0, 0, > > + 0, 0, 0, (T){}, > > + (T){}, (W){}, > > + (W){}, (T){}, (T){}, 0, 0, 0, 0, 0, 0, (W){}, > > + (V){}, (W){}, (T){}, 0, 0, (U){}, (F){}); > > +} >
On Wed, 13 Apr 2022, Richard Biener wrote: > On Wed, 13 Apr 2022, Richard Sandiford wrote: > > > Richard Biener via Gcc-patches <gcc-patches@gcc.gnu.org> writes: > > > The following reverts the original PR105140 fix and goes for instead > > > applying the additional fold_convert constraint for VECTOR_TYPE > > > conversions also to fold_convertible_p. I did not try sanitizing > > > all of this at this point. > > > > > > Bootstrapped on x86_64-unknown-linux-gnu, testing in progress. > > > > > > 2022-04-13 Richard Biener <rguenther@suse.de> > > > > > > PR tree-optimization/105250 > > > * fold-const.cc (fold_convertible_p): Revert > > > r12-7979-geaaf77dd85c333, instead check for size equality > > > of the vector types involved. > > > > This doesn't look right, and I think it'll break SVE. For one > > thing, the tree_int_cst_equal check is bound to fail for > > variable-length vectors. > > > > But also, the idea was to allow element-wise conversions between > > different vector sizes. For example, you can do a nop/convert > > from V4SI to V4DI, which converts 4 SIs to 4 DIs. This is used > > a lot for conversions to and from “partial” SVE vectors, where smaller > > elements are stored in wider containers. > > But fold_convertible_p is used as guard for fold_convert in a lot of > places and that will simply ICE when there's a mismatch in size > as can be seen in the testcase. Note the code as before the > previous fix couldn't really have worked as expected. Is there any > testcase that will "break" now? > > I realize the fold_convertible_p comment says "using a NOP_EXPR" which > means it might conver a narrower set of conversions than fold_convert > (which will happily use FLOAT_EXPR and friends), but still it should > allow fold_convert to build the conversion. > > The alternative would have been to emit a NOP_EXPR from fold_convert > for vector type conversions (with the correct constraints), but then > not all targets support those, so we'd need a target support check > in fold_convertible_p then? Btw, fold_convertible_p is currently used in few places only: fold-const.cc:fold_convertible_p (const_tree type, const_tree arg) ipa-cp.cc: || fold_convertible_p (param_type, value)) ipa-param-manipulation.cc: if (!fold_convertible_p (TREE_TYPE (origin), arg)) ipa-prop.cc: if (fold_convertible_p (TREE_TYPE (rhs), v->value)) tree-inline.cc: if (fold_convertible_p (type, value)) tree-inline.cc: if (fold_convertible_p (caller_type, var)) all in places that try to deal with type mismatches in IPA (from parameters). Richard. > Richard. > > > Thanks, > > Richard > > > > > > > > * gcc.dg/pr105250.c: New testcase. > > > --- > > > gcc/fold-const.cc | 7 +++---- > > > gcc/testsuite/gcc.dg/pr105250.c | 29 +++++++++++++++++++++++++++++ > > > 2 files changed, 32 insertions(+), 4 deletions(-) > > > create mode 100644 gcc/testsuite/gcc.dg/pr105250.c > > > > > > diff --git a/gcc/fold-const.cc b/gcc/fold-const.cc > > > index 7226bc5af01..a57ad0739fb 100644 > > > --- a/gcc/fold-const.cc > > > +++ b/gcc/fold-const.cc > > > @@ -2379,13 +2379,12 @@ build_zero_vector (tree type) > > > return build_vector_from_val (type, t); > > > } > > > > > > -/* Returns true, if ARG, an operand or a type, is convertible to TYPE > > > - using a NOP_EXPR. */ > > > +/* Returns true, if ARG is convertible to TYPE using a NOP_EXPR. */ > > > > > > bool > > > fold_convertible_p (const_tree type, const_tree arg) > > > { > > > - const_tree orig = TYPE_P (arg) ? arg : TREE_TYPE (arg); > > > + const_tree orig = TREE_TYPE (arg); > > > > > > if (type == orig) > > > return true; > > > @@ -2417,7 +2416,7 @@ fold_convertible_p (const_tree type, const_tree arg) > > > return (VECTOR_TYPE_P (orig) > > > && known_eq (TYPE_VECTOR_SUBPARTS (type), > > > TYPE_VECTOR_SUBPARTS (orig)) > > > - && fold_convertible_p (TREE_TYPE (type), TREE_TYPE (orig))); > > > + && tree_int_cst_equal (TYPE_SIZE (type), TYPE_SIZE (orig))); > > > > > > default: > > > return false; > > > diff --git a/gcc/testsuite/gcc.dg/pr105250.c b/gcc/testsuite/gcc.dg/pr105250.c > > > new file mode 100644 > > > index 00000000000..665dd95d8cb > > > --- /dev/null > > > +++ b/gcc/testsuite/gcc.dg/pr105250.c > > > @@ -0,0 +1,29 @@ > > > +/* { dg-do compile } */ > > > +/* { dg-options "-w -Wno-psabi -O2" } */ > > > + > > > +typedef int __attribute__((__vector_size__(4))) T; > > > +typedef int __attribute__((__vector_size__(8))) U; > > > +typedef int __attribute__((__vector_size__(16))) V; > > > +typedef int __attribute__((__vector_size__(32))) W; > > > +typedef _Float32 __attribute__((__vector_size__(16))) F; > > > +typedef _Float64 __attribute__((__vector_size__(32))) G; > > > +void foo(); > > > + > > > +foo(int, int, int, int, U, U, V, V, W, W, int, > > > + T, int, U, U, V, V, W, W, T, > > > + T, int, U, U, V, V, W, W, T, > > > + T, int, W, W, T, T, int, int, int, > > > + int, int, int, W, int, int, int, int, int, int, > > > + V, W, T, int, int, U, F, int, int, int, > > > + int, int, int, G) > > > +{ > > > + foo(0, 0, 0, 0, (U){}, (U){}, (V){}, (V){}, (W){}, > > > + (W){}, 2, (T){}, 0, 0, 0, 0, (U){}, (U){}, > > > + (V){}, (V){}, (W){}, (W){}, (T){}, > > > + (T){}, 0, 0, 0, 0, (U){}, (U){}, (V){}, > > > + (V){}, (W){}, (W){}, (T){}, (T){}, 0, 0, 0, > > > + 0, 0, 0, (T){}, > > > + (T){}, (W){}, > > > + (W){}, (T){}, (T){}, 0, 0, 0, 0, 0, 0, (W){}, > > > + (V){}, (W){}, (T){}, 0, 0, (U){}, (F){}); > > > +} > > > >
On Wed, 13 Apr 2022, Richard Biener wrote: > On Wed, 13 Apr 2022, Richard Biener wrote: > > > On Wed, 13 Apr 2022, Richard Sandiford wrote: > > > > > Richard Biener via Gcc-patches <gcc-patches@gcc.gnu.org> writes: > > > > The following reverts the original PR105140 fix and goes for instead > > > > applying the additional fold_convert constraint for VECTOR_TYPE > > > > conversions also to fold_convertible_p. I did not try sanitizing > > > > all of this at this point. > > > > > > > > Bootstrapped on x86_64-unknown-linux-gnu, testing in progress. > > > > > > > > 2022-04-13 Richard Biener <rguenther@suse.de> > > > > > > > > PR tree-optimization/105250 > > > > * fold-const.cc (fold_convertible_p): Revert > > > > r12-7979-geaaf77dd85c333, instead check for size equality > > > > of the vector types involved. > > > > > > This doesn't look right, and I think it'll break SVE. For one > > > thing, the tree_int_cst_equal check is bound to fail for > > > variable-length vectors. > > > > > > But also, the idea was to allow element-wise conversions between > > > different vector sizes. For example, you can do a nop/convert > > > from V4SI to V4DI, which converts 4 SIs to 4 DIs. This is used > > > a lot for conversions to and from “partial” SVE vectors, where smaller > > > elements are stored in wider containers. > > > > But fold_convertible_p is used as guard for fold_convert in a lot of > > places and that will simply ICE when there's a mismatch in size > > as can be seen in the testcase. Note the code as before the > > previous fix couldn't really have worked as expected. Is there any > > testcase that will "break" now? > > > > I realize the fold_convertible_p comment says "using a NOP_EXPR" which > > means it might conver a narrower set of conversions than fold_convert > > (which will happily use FLOAT_EXPR and friends), but still it should > > allow fold_convert to build the conversion. > > > > The alternative would have been to emit a NOP_EXPR from fold_convert > > for vector type conversions (with the correct constraints), but then > > not all targets support those, so we'd need a target support check > > in fold_convertible_p then? > > Btw, fold_convertible_p is currently used in few places only: > > fold-const.cc:fold_convertible_p (const_tree type, const_tree arg) > ipa-cp.cc: || fold_convertible_p (param_type, value)) > ipa-param-manipulation.cc: if (!fold_convertible_p (TREE_TYPE > (origin), arg)) > ipa-prop.cc: if (fold_convertible_p (TREE_TYPE (rhs), v->value)) > tree-inline.cc: if (fold_convertible_p (type, value)) > tree-inline.cc: if (fold_convertible_p (caller_type, var)) > > all in places that try to deal with type mismatches in IPA (from > parameters). Btw, I can't find a tree_int_cst_equal replacement that would work for POLY_INT_CST as well as INTEGER_CST, is there any that I missed? Richard. > Richard. > > > Richard. > > > > > Thanks, > > > Richard > > > > > > > > > > > * gcc.dg/pr105250.c: New testcase. > > > > --- > > > > gcc/fold-const.cc | 7 +++---- > > > > gcc/testsuite/gcc.dg/pr105250.c | 29 +++++++++++++++++++++++++++++ > > > > 2 files changed, 32 insertions(+), 4 deletions(-) > > > > create mode 100644 gcc/testsuite/gcc.dg/pr105250.c > > > > > > > > diff --git a/gcc/fold-const.cc b/gcc/fold-const.cc > > > > index 7226bc5af01..a57ad0739fb 100644 > > > > --- a/gcc/fold-const.cc > > > > +++ b/gcc/fold-const.cc > > > > @@ -2379,13 +2379,12 @@ build_zero_vector (tree type) > > > > return build_vector_from_val (type, t); > > > > } > > > > > > > > -/* Returns true, if ARG, an operand or a type, is convertible to TYPE > > > > - using a NOP_EXPR. */ > > > > +/* Returns true, if ARG is convertible to TYPE using a NOP_EXPR. */ > > > > > > > > bool > > > > fold_convertible_p (const_tree type, const_tree arg) > > > > { > > > > - const_tree orig = TYPE_P (arg) ? arg : TREE_TYPE (arg); > > > > + const_tree orig = TREE_TYPE (arg); > > > > > > > > if (type == orig) > > > > return true; > > > > @@ -2417,7 +2416,7 @@ fold_convertible_p (const_tree type, const_tree arg) > > > > return (VECTOR_TYPE_P (orig) > > > > && known_eq (TYPE_VECTOR_SUBPARTS (type), > > > > TYPE_VECTOR_SUBPARTS (orig)) > > > > - && fold_convertible_p (TREE_TYPE (type), TREE_TYPE (orig))); > > > > + && tree_int_cst_equal (TYPE_SIZE (type), TYPE_SIZE (orig))); > > > > > > > > default: > > > > return false; > > > > diff --git a/gcc/testsuite/gcc.dg/pr105250.c b/gcc/testsuite/gcc.dg/pr105250.c > > > > new file mode 100644 > > > > index 00000000000..665dd95d8cb > > > > --- /dev/null > > > > +++ b/gcc/testsuite/gcc.dg/pr105250.c > > > > @@ -0,0 +1,29 @@ > > > > +/* { dg-do compile } */ > > > > +/* { dg-options "-w -Wno-psabi -O2" } */ > > > > + > > > > +typedef int __attribute__((__vector_size__(4))) T; > > > > +typedef int __attribute__((__vector_size__(8))) U; > > > > +typedef int __attribute__((__vector_size__(16))) V; > > > > +typedef int __attribute__((__vector_size__(32))) W; > > > > +typedef _Float32 __attribute__((__vector_size__(16))) F; > > > > +typedef _Float64 __attribute__((__vector_size__(32))) G; > > > > +void foo(); > > > > + > > > > +foo(int, int, int, int, U, U, V, V, W, W, int, > > > > + T, int, U, U, V, V, W, W, T, > > > > + T, int, U, U, V, V, W, W, T, > > > > + T, int, W, W, T, T, int, int, int, > > > > + int, int, int, W, int, int, int, int, int, int, > > > > + V, W, T, int, int, U, F, int, int, int, > > > > + int, int, int, G) > > > > +{ > > > > + foo(0, 0, 0, 0, (U){}, (U){}, (V){}, (V){}, (W){}, > > > > + (W){}, 2, (T){}, 0, 0, 0, 0, (U){}, (U){}, > > > > + (V){}, (V){}, (W){}, (W){}, (T){}, > > > > + (T){}, 0, 0, 0, 0, (U){}, (U){}, (V){}, > > > > + (V){}, (W){}, (W){}, (T){}, (T){}, 0, 0, 0, > > > > + 0, 0, 0, (T){}, > > > > + (T){}, (W){}, > > > > + (W){}, (T){}, (T){}, 0, 0, 0, 0, 0, 0, (W){}, > > > > + (V){}, (W){}, (T){}, 0, 0, (U){}, (F){}); > > > > +} > > > > > > > > >
Richard Biener <rguenther@suse.de> writes: > On Wed, 13 Apr 2022, Richard Sandiford wrote: > >> Richard Biener via Gcc-patches <gcc-patches@gcc.gnu.org> writes: >> > The following reverts the original PR105140 fix and goes for instead >> > applying the additional fold_convert constraint for VECTOR_TYPE >> > conversions also to fold_convertible_p. I did not try sanitizing >> > all of this at this point. >> > >> > Bootstrapped on x86_64-unknown-linux-gnu, testing in progress. >> > >> > 2022-04-13 Richard Biener <rguenther@suse.de> >> > >> > PR tree-optimization/105250 >> > * fold-const.cc (fold_convertible_p): Revert >> > r12-7979-geaaf77dd85c333, instead check for size equality >> > of the vector types involved. >> >> This doesn't look right, and I think it'll break SVE. For one >> thing, the tree_int_cst_equal check is bound to fail for >> variable-length vectors. >> >> But also, the idea was to allow element-wise conversions between >> different vector sizes. For example, you can do a nop/convert >> from V4SI to V4DI, which converts 4 SIs to 4 DIs. This is used >> a lot for conversions to and from “partial” SVE vectors, where smaller >> elements are stored in wider containers. > > But fold_convertible_p is used as guard for fold_convert in a lot of > places and that will simply ICE when there's a mismatch in size > as can be seen in the testcase. Note the code as before the > previous fix couldn't really have worked as expected. Is there any > testcase that will "break" now? > > I realize the fold_convertible_p comment says "using a NOP_EXPR" which > means it might conver a narrower set of conversions than fold_convert > (which will happily use FLOAT_EXPR and friends), but still it should > allow fold_convert to build the conversion. Ah, right, sorry for the false alarm. I'd read the comment as saying that it determined when nop/convert_exprs were valid, rather than whether fold_convert could handle them. > The alternative would have been to emit a NOP_EXPR from fold_convert > for vector type conversions (with the correct constraints), but then > not all targets support those, so we'd need a target support check > in fold_convertible_p then? Yeah, I guess it makes sense to keep things as they are, with code that is aware of these vector conversions using supportable_convert_operation. It tests fine on SVE, as you expected. Thanks, Richard
diff --git a/gcc/fold-const.cc b/gcc/fold-const.cc index 7226bc5af01..a57ad0739fb 100644 --- a/gcc/fold-const.cc +++ b/gcc/fold-const.cc @@ -2379,13 +2379,12 @@ build_zero_vector (tree type) return build_vector_from_val (type, t); } -/* Returns true, if ARG, an operand or a type, is convertible to TYPE - using a NOP_EXPR. */ +/* Returns true, if ARG is convertible to TYPE using a NOP_EXPR. */ bool fold_convertible_p (const_tree type, const_tree arg) { - const_tree orig = TYPE_P (arg) ? arg : TREE_TYPE (arg); + const_tree orig = TREE_TYPE (arg); if (type == orig) return true; @@ -2417,7 +2416,7 @@ fold_convertible_p (const_tree type, const_tree arg) return (VECTOR_TYPE_P (orig) && known_eq (TYPE_VECTOR_SUBPARTS (type), TYPE_VECTOR_SUBPARTS (orig)) - && fold_convertible_p (TREE_TYPE (type), TREE_TYPE (orig))); + && tree_int_cst_equal (TYPE_SIZE (type), TYPE_SIZE (orig))); default: return false; diff --git a/gcc/testsuite/gcc.dg/pr105250.c b/gcc/testsuite/gcc.dg/pr105250.c new file mode 100644 index 00000000000..665dd95d8cb --- /dev/null +++ b/gcc/testsuite/gcc.dg/pr105250.c @@ -0,0 +1,29 @@ +/* { dg-do compile } */ +/* { dg-options "-w -Wno-psabi -O2" } */ + +typedef int __attribute__((__vector_size__(4))) T; +typedef int __attribute__((__vector_size__(8))) U; +typedef int __attribute__((__vector_size__(16))) V; +typedef int __attribute__((__vector_size__(32))) W; +typedef _Float32 __attribute__((__vector_size__(16))) F; +typedef _Float64 __attribute__((__vector_size__(32))) G; +void foo(); + +foo(int, int, int, int, U, U, V, V, W, W, int, + T, int, U, U, V, V, W, W, T, + T, int, U, U, V, V, W, W, T, + T, int, W, W, T, T, int, int, int, + int, int, int, W, int, int, int, int, int, int, + V, W, T, int, int, U, F, int, int, int, + int, int, int, G) +{ + foo(0, 0, 0, 0, (U){}, (U){}, (V){}, (V){}, (W){}, + (W){}, 2, (T){}, 0, 0, 0, 0, (U){}, (U){}, + (V){}, (V){}, (W){}, (W){}, (T){}, + (T){}, 0, 0, 0, 0, (U){}, (U){}, (V){}, + (V){}, (W){}, (W){}, (T){}, (T){}, 0, 0, 0, + 0, 0, 0, (T){}, + (T){}, (W){}, + (W){}, (T){}, (T){}, 0, 0, 0, 0, 0, 0, (W){}, + (V){}, (W){}, (T){}, 0, 0, (U){}, (F){}); +}