From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx2.suse.de (mx2.suse.de [195.135.220.15]) by sourceware.org (Postfix) with ESMTPS id AC2D9396E840 for ; Thu, 7 Jan 2021 08:40:55 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org AC2D9396E840 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=suse.de Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=rguenther@suse.de X-Virus-Scanned: by amavisd-new at test-mx.suse.de Received: from relay2.suse.de (unknown [195.135.221.27]) by mx2.suse.de (Postfix) with ESMTP id 93E23AD1E; Thu, 7 Jan 2021 08:40:54 +0000 (UTC) Date: Thu, 7 Jan 2021 09:40:54 +0100 (CET) From: Richard Biener To: Jakub Jelinek cc: gcc-patches@gcc.gnu.org Subject: Re: [PATCH] bswap: Fix up recent vector CONSTRUCTOR optimization [PR98568] In-Reply-To: <20210107083147.GK725145@tucnak> Message-ID: References: <20210107083147.GK725145@tucnak> User-Agent: Alpine 2.21 (LSU 202 2017-01-01) MIME-Version: 1.0 X-Spam-Status: No, score=-5.2 required=5.0 tests=BAYES_00, KAM_DMARC_STATUS, RCVD_IN_MSPIKE_H3, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org Content-Type: text/plain; charset=ISO-8859-15 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: Thu, 07 Jan 2021 08:40:57 -0000 On Thu, 7 Jan 2021, Jakub Jelinek wrote: > Hi! > > As the testcase shows, bswap can match even byte-swapping or indentity > from low part of some wider SSA_NAME. > For bswap replacement other than for vector CONSTRUCTOR the code has been > using NOP_EXPR casts if the types weren't compatible, but for vectors > we need to use VIEW_CONVERT_EXPR. The problem with the latter is that > we require that it has the same size, which isn't guaranteed, so this patch > in those cases first adds a narrowing NOP_EXPR cast and only afterwards > does a VIEW_CONVERT_EXPR. > > Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux, ok for > trunk? OK. Thanks, Richard. > 2021-01-06 Jakub Jelinek > > PR tree-optimization/98568 > * gimple-ssa-store-merging.c (bswap_view_convert): New function. > (bswap_replace): Use it. > > * g++.dg/torture/pr98568.C: New test. > > --- gcc/gimple-ssa-store-merging.c.jj 2021-01-05 16:15:58.965337667 +0100 > +++ gcc/gimple-ssa-store-merging.c 2021-01-06 19:20:37.225578412 +0100 > @@ -978,6 +978,25 @@ public: > > }; // class pass_optimize_bswap > > +/* Helper function for bswap_replace. Build VIEW_CONVERT_EXPR from > + VAL to TYPE. If VAL has different type size, emit a NOP_EXPR cast > + first. */ > + > +static tree > +bswap_view_convert (gimple_stmt_iterator *gsi, tree type, tree val) > +{ > + gcc_assert (INTEGRAL_TYPE_P (TREE_TYPE (val))); > + if (TYPE_SIZE (type) != TYPE_SIZE (TREE_TYPE (val))) > + { > + HOST_WIDE_INT prec = TREE_INT_CST_LOW (TYPE_SIZE (type)); > + tree itype = build_nonstandard_integer_type (prec, 1); > + gimple *g = gimple_build_assign (make_ssa_name (itype), NOP_EXPR, val); > + gsi_insert_before (gsi, g, GSI_SAME_STMT); > + val = gimple_assign_lhs (g); > + } > + return build1 (VIEW_CONVERT_EXPR, type, val); > +} > + > /* Perform the bswap optimization: replace the expression computed in the rhs > of gsi_stmt (GSI) (or if NULL add instead of replace) by an equivalent > bswap, load or load + bswap expression. > @@ -1100,7 +1119,7 @@ bswap_replace (gimple_stmt_iterator gsi, > gimple_set_vuse (load_stmt, n->vuse); > gsi_insert_before (&gsi, load_stmt, GSI_SAME_STMT); > if (conv_code == VIEW_CONVERT_EXPR) > - val_tmp = build1 (VIEW_CONVERT_EXPR, TREE_TYPE (tgt), val_tmp); > + val_tmp = bswap_view_convert (&gsi, TREE_TYPE (tgt), val_tmp); > gimple_assign_set_rhs_with_ops (&gsi, conv_code, val_tmp); > update_stmt (cur_stmt); > } > @@ -1144,7 +1163,7 @@ bswap_replace (gimple_stmt_iterator gsi, > if (!is_gimple_val (src)) > return NULL_TREE; > if (conv_code == VIEW_CONVERT_EXPR) > - src = build1 (VIEW_CONVERT_EXPR, TREE_TYPE (tgt), src); > + src = bswap_view_convert (&gsi, TREE_TYPE (tgt), src); > g = gimple_build_assign (tgt, conv_code, src); > } > else if (cur_stmt) > @@ -1227,7 +1246,7 @@ bswap_replace (gimple_stmt_iterator gsi, > tmp = make_temp_ssa_name (bswap_type, NULL, "bswapdst"); > tree atmp = tmp; > if (conv_code == VIEW_CONVERT_EXPR) > - atmp = build1 (VIEW_CONVERT_EXPR, TREE_TYPE (tgt), tmp); > + atmp = bswap_view_convert (&gsi, TREE_TYPE (tgt), tmp); > convert_stmt = gimple_build_assign (tgt, conv_code, atmp); > gsi_insert_after (&gsi, convert_stmt, GSI_SAME_STMT); > } > --- gcc/testsuite/g++.dg/torture/pr98568.C.jj 2021-01-06 19:19:25.940377456 +0100 > +++ gcc/testsuite/g++.dg/torture/pr98568.C 2021-01-06 19:19:16.608482074 +0100 > @@ -0,0 +1,37 @@ > +// PR tree-optimization/98568 > +// { dg-do compile } > + > +char a[2]; > +char b[4]; > + > +void > +foo (int x) > +{ > + a[1] = x >> 8; > + a[0] = x; > +} > + > +void > +bar (long long x) > +{ > + b[3] = x >> 24; > + b[2] = x >> 16; > + b[1] = x >> 8; > + b[0] = x; > +} > + > +void > +baz (int x) > +{ > + a[0] = x >> 8; > + a[1] = x; > +} > + > +void > +qux (long long x) > +{ > + b[0] = x >> 24; > + b[1] = x >> 16; > + b[2] = x >> 8; > + b[3] = x; > +} > > Jakub > > -- Richard Biener SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg, Germany; GF: Felix Imendörffer; HRB 36809 (AG Nuernberg)