Message ID | 02ee1acd-1951-8cf9-345b-ca230ae432bb@codesourcery.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 8F23E385AC19 for <patchwork@sourceware.org>; Fri, 29 Jul 2022 15:54:13 +0000 (GMT) X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from esa1.mentor.iphmx.com (esa1.mentor.iphmx.com [68.232.129.153]) by sourceware.org (Postfix) with ESMTPS id 782583858297 for <gcc-patches@gcc.gnu.org>; Fri, 29 Jul 2022 15:53:57 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 782583858297 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=codesourcery.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=mentor.com X-IronPort-AV: E=Sophos;i="5.93,201,1654588800"; d="scan'208";a="83209541" Received: from orw-gwy-01-in.mentorg.com ([192.94.38.165]) by esa1.mentor.iphmx.com with ESMTP; 29 Jul 2022 07:53:56 -0800 IronPort-SDR: xhUzc77eSKpsz9UxFEn9u9hQBNvyBLBPwyOnaUG5rE0wTuGe+5rWRA6cx77XhYAVPY+82Kpu+7 EMmth9PsDtrcNGgS+otq6D7qogRagHM8kduvmXh+oVt5t2+Jsa6LIEtoBR4awvwadTodj2lZ11 9mDI0ptLWPsbGWs5uk5KR+Uqu4TGPD6NJefFF6AAxOC0aoJ305abgf92jRWeK3CNEPUOEbi7Xl aCT8TH876LXtYZHqSGO2EVI5KAlqRNZfXGKBW2Maxiwi7rqOoB2MMUMV4J7I0FH9XgdO3JCYGD dh0= Content-Type: multipart/mixed; boundary="------------T1trZyyJmseEC3yhVde6V0z0" Message-ID: <02ee1acd-1951-8cf9-345b-ca230ae432bb@codesourcery.com> Date: Fri, 29 Jul 2022 16:53:51 +0100 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101 Thunderbird/102.0.3 Content-Language: en-GB To: "gcc-patches@gcc.gnu.org" <gcc-patches@gcc.gnu.org> From: Andrew Stubbs <ams@codesourcery.com> Subject: [PATCH] openmp-simd-clone: Match shift type X-Originating-IP: [137.202.0.90] X-ClientProxiedBy: svr-ies-mbx-15.mgc.mentorg.com (139.181.222.15) To svr-ies-mbx-11.mgc.mentorg.com (139.181.222.11) X-Spam-Status: No, score=-11.5 required=5.0 tests=BAYES_00, GIT_PATCH_0, HEADER_FROM_DIFFERENT_DOMAINS, KAM_DMARC_STATUS, SPF_HELO_PASS, SPF_PASS, 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> Errors-To: gcc-patches-bounces+patchwork=sourceware.org@gcc.gnu.org Sender: "Gcc-patches" <gcc-patches-bounces+patchwork=sourceware.org@gcc.gnu.org> |
Series |
openmp-simd-clone: Match shift type
|
|
Commit Message
Andrew Stubbs
July 29, 2022, 3:53 p.m. UTC
This patch adjusts the generation of SIMD "inbranch" clones that use integer masks to ensure that it vectorizes on amdgcn. The problem was only that an amdgcn mask is DImode and the shift amount was SImode, and the difference causes vectorization to fail. OK for mainline? Andrew openmp-simd-clone: Match shift types Ensure that both parameters to vector shifts use the same mode. This is most important for amdgcn where the masks are DImode. gcc/ChangeLog: * omp-simd-clone.cc (simd_clone_adjust): Convert shift_cnt to match the mask type.
Comments
On Fri, Jul 29, 2022 at 04:53:51PM +0100, Andrew Stubbs wrote: > This patch adjusts the generation of SIMD "inbranch" clones that use integer > masks to ensure that it vectorizes on amdgcn. > > The problem was only that an amdgcn mask is DImode and the shift amount was > SImode, and the difference causes vectorization to fail. > > OK for mainline? > > Andrew > openmp-simd-clone: Match shift types > > Ensure that both parameters to vector shifts use the same mode. This is most > important for amdgcn where the masks are DImode. > > gcc/ChangeLog: > > * omp-simd-clone.cc (simd_clone_adjust): Convert shift_cnt to match > the mask type. > > diff --git a/gcc/omp-simd-clone.cc b/gcc/omp-simd-clone.cc > index 32649bc3f9a..5d3a90730e7 100644 > --- a/gcc/omp-simd-clone.cc > +++ b/gcc/omp-simd-clone.cc > @@ -1305,8 +1305,12 @@ simd_clone_adjust (struct cgraph_node *node) > build_int_cst (TREE_TYPE (iter1), c)); > gsi_insert_after (&gsi, g, GSI_CONTINUE_LINKING); > } > + tree shift_cnt_conv = make_ssa_name (TREE_TYPE (mask)); > + g = gimple_build_assign (shift_cnt_conv, > + fold_convert (TREE_TYPE (mask), shift_cnt)); > + gsi_insert_after (&gsi, g, GSI_CONTINUE_LINKING); Doing the fold_convert seems to be a wasted effort to me. Can't this be done conditional on whether some change is needed at all and just using gimple_build_assign with NOP_EXPR, so something like: tree shift_cvt_conv = shift_cnt; if (!useless_type_conversion_p (TREE_TYPE (mask), TREE_TYPE (shift_cnt))) { shift_cnt_conv = make_ssa_name (TREE_TYPE (mask)); g = gimple_build_assign (shift_cnt_conv, NOP_EXPR, shift_cnt); gsi_insert_after (&gsi, g, GSI_CONTINUE_LINKING); } > g = gimple_build_assign (make_ssa_name (TREE_TYPE (mask)), > - RSHIFT_EXPR, mask, shift_cnt); > + RSHIFT_EXPR, mask, shift_cnt_conv); > gsi_insert_after (&gsi, g, GSI_CONTINUE_LINKING); > mask = gimple_assign_lhs (g); > g = gimple_build_assign (make_ssa_name (TREE_TYPE (mask)), ? Jakub
On 29/07/2022 16:59, Jakub Jelinek wrote: > Doing the fold_convert seems to be a wasted effort to me. > Can't this be done conditional on whether some change is needed at all > and just using gimple_build_assign with NOP_EXPR, so something like: I'm just not familiar enough with this stuff to run fold_convert in my head with confidence. > tree shift_cvt_conv = shift_cnt; > if (!useless_type_conversion_p (TREE_TYPE (mask), > TREE_TYPE (shift_cnt))) > { > shift_cnt_conv = make_ssa_name (TREE_TYPE (mask)); > g = gimple_build_assign (shift_cnt_conv, NOP_EXPR, shift_cnt); > gsi_insert_after (&gsi, g, GSI_CONTINUE_LINKING); > } > Your version gives the same output mine does, at least on amdgcn anyway. Am I OK to commit this version? Andrew openmp-simd-clone: Match shift types Ensure that both parameters to vector shifts use the same mode. This is most important for amdgcn where the masks are DImode. gcc/ChangeLog: * omp-simd-clone.cc (simd_clone_adjust): Convert shift_cnt to match the mask type. Co-authored-by: Jakub Jelinek <jakub@redhat.com> diff --git a/gcc/omp-simd-clone.cc b/gcc/omp-simd-clone.cc index 32649bc3f9a..58bd68b129b 100644 --- a/gcc/omp-simd-clone.cc +++ b/gcc/omp-simd-clone.cc @@ -1305,8 +1305,16 @@ simd_clone_adjust (struct cgraph_node *node) build_int_cst (TREE_TYPE (iter1), c)); gsi_insert_after (&gsi, g, GSI_CONTINUE_LINKING); } + tree shift_cnt_conv = shift_cnt; + if (!useless_type_conversion_p (TREE_TYPE (mask), + TREE_TYPE (shift_cnt))) + { + shift_cnt_conv = make_ssa_name (TREE_TYPE (mask)); + g = gimple_build_assign (shift_cnt_conv, NOP_EXPR, shift_cnt); + gsi_insert_after (&gsi, g, GSI_CONTINUE_LINKING); + } g = gimple_build_assign (make_ssa_name (TREE_TYPE (mask)), - RSHIFT_EXPR, mask, shift_cnt); + RSHIFT_EXPR, mask, shift_cnt_conv); gsi_insert_after (&gsi, g, GSI_CONTINUE_LINKING); mask = gimple_assign_lhs (g); g = gimple_build_assign (make_ssa_name (TREE_TYPE (mask)),
On Fri, Jul 29, 2022 at 06:03:18PM +0100, Andrew Stubbs wrote: > On 29/07/2022 16:59, Jakub Jelinek wrote: > > Doing the fold_convert seems to be a wasted effort to me. > > Can't this be done conditional on whether some change is needed at all > > and just using gimple_build_assign with NOP_EXPR, so something like: > > I'm just not familiar enough with this stuff to run fold_convert in my head > with confidence. The thing with fold_convert is that if some conversion is needed (and fold_convert actually is strict, so even if the conversion is useless but the type isn't exactly the same) it creates a NOP_EXPR around the argument, and then gimple_build_assign notices it should create a NOP_EXPR assign rhs op and just uses the argument of NOP_EXPR, where the NOP_EXPR will be GC later. Plus, if the conversion isn't needed, it creates an extra assignment that will be only later in some other pass optimized away. > > > tree shift_cvt_conv = shift_cnt; > > if (!useless_type_conversion_p (TREE_TYPE (mask), > > TREE_TYPE (shift_cnt))) > > { > > shift_cnt_conv = make_ssa_name (TREE_TYPE (mask)); > > g = gimple_build_assign (shift_cnt_conv, NOP_EXPR, shift_cnt); > > gsi_insert_after (&gsi, g, GSI_CONTINUE_LINKING); > > } > > > > Your version gives the same output mine does, at least on amdgcn anyway. > > Am I OK to commit this version? Yes, thanks. > openmp-simd-clone: Match shift types > > Ensure that both parameters to vector shifts use the same mode. This is most > important for amdgcn where the masks are DImode. > > gcc/ChangeLog: > > * omp-simd-clone.cc (simd_clone_adjust): Convert shift_cnt to match > the mask type. > > Co-authored-by: Jakub Jelinek <jakub@redhat.com> > > diff --git a/gcc/omp-simd-clone.cc b/gcc/omp-simd-clone.cc > index 32649bc3f9a..58bd68b129b 100644 > --- a/gcc/omp-simd-clone.cc > +++ b/gcc/omp-simd-clone.cc > @@ -1305,8 +1305,16 @@ simd_clone_adjust (struct cgraph_node *node) > build_int_cst (TREE_TYPE (iter1), c)); > gsi_insert_after (&gsi, g, GSI_CONTINUE_LINKING); > } > + tree shift_cnt_conv = shift_cnt; > + if (!useless_type_conversion_p (TREE_TYPE (mask), > + TREE_TYPE (shift_cnt))) > + { > + shift_cnt_conv = make_ssa_name (TREE_TYPE (mask)); > + g = gimple_build_assign (shift_cnt_conv, NOP_EXPR, shift_cnt); > + gsi_insert_after (&gsi, g, GSI_CONTINUE_LINKING); > + } > g = gimple_build_assign (make_ssa_name (TREE_TYPE (mask)), > - RSHIFT_EXPR, mask, shift_cnt); > + RSHIFT_EXPR, mask, shift_cnt_conv); > gsi_insert_after (&gsi, g, GSI_CONTINUE_LINKING); > mask = gimple_assign_lhs (g); > g = gimple_build_assign (make_ssa_name (TREE_TYPE (mask)), Jakub
diff --git a/gcc/omp-simd-clone.cc b/gcc/omp-simd-clone.cc index 32649bc3f9a..5d3a90730e7 100644 --- a/gcc/omp-simd-clone.cc +++ b/gcc/omp-simd-clone.cc @@ -1305,8 +1305,12 @@ simd_clone_adjust (struct cgraph_node *node) build_int_cst (TREE_TYPE (iter1), c)); gsi_insert_after (&gsi, g, GSI_CONTINUE_LINKING); } + tree shift_cnt_conv = make_ssa_name (TREE_TYPE (mask)); + g = gimple_build_assign (shift_cnt_conv, + fold_convert (TREE_TYPE (mask), shift_cnt)); + gsi_insert_after (&gsi, g, GSI_CONTINUE_LINKING); g = gimple_build_assign (make_ssa_name (TREE_TYPE (mask)), - RSHIFT_EXPR, mask, shift_cnt); + RSHIFT_EXPR, mask, shift_cnt_conv); gsi_insert_after (&gsi, g, GSI_CONTINUE_LINKING); mask = gimple_assign_lhs (g); g = gimple_build_assign (make_ssa_name (TREE_TYPE (mask)),