From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp-out2.suse.de (smtp-out2.suse.de [195.135.220.29]) by sourceware.org (Postfix) with ESMTPS id CEAEF385840F for ; Wed, 13 Apr 2022 09:06:42 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org CEAEF385840F Received: from relay2.suse.de (relay2.suse.de [149.44.160.134]) by smtp-out2.suse.de (Postfix) with ESMTP id BF29C1F856; Wed, 13 Apr 2022 09:06:41 +0000 (UTC) Received: from murzim.suse.de (murzim.suse.de [10.160.4.192]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by relay2.suse.de (Postfix) with ESMTPS id B86FBA3B93; Wed, 13 Apr 2022 09:06:41 +0000 (UTC) Date: Wed, 13 Apr 2022 11:06:41 +0200 (CEST) From: Richard Biener To: Richard Sandiford cc: Richard Biener via Gcc-patches Subject: Re: [PATCH] tree-optimization/105250 - adjust fold_convertible_p PR105140 fix In-Reply-To: Message-ID: <1sq73r63-p556-q089-9psn-o03n6q95632@fhfr.qr> References: <20220413074636.9269E13A91@imap2.suse-dmz.suse.de> MIME-Version: 1.0 X-Spam-Status: No, score=-10.9 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, 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 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8BIT X-Content-Filtered-By: Mailman/MimeDel 2.1.29 X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 13 Apr 2022 09:06:44 -0000 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 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 > > > > > > > > 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 SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg, Germany; GF: Ivo Totev; HRB 36809 (AG Nuernberg)