* [PATCH] openmp-simd-clone: Match shift type @ 2022-07-29 15:53 Andrew Stubbs 2022-07-29 15:59 ` Jakub Jelinek 0 siblings, 1 reply; 4+ messages in thread From: Andrew Stubbs @ 2022-07-29 15:53 UTC (permalink / raw) To: gcc-patches [-- Attachment #1: Type: text/plain, Size: 284 bytes --] 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 [-- Attachment #2: 220729-simd-clone-shift-types.patch --] [-- Type: text/plain, Size: 1115 bytes --] 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); 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)), ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] openmp-simd-clone: Match shift type 2022-07-29 15:53 [PATCH] openmp-simd-clone: Match shift type Andrew Stubbs @ 2022-07-29 15:59 ` Jakub Jelinek 2022-07-29 17:03 ` Andrew Stubbs 0 siblings, 1 reply; 4+ messages in thread From: Jakub Jelinek @ 2022-07-29 15:59 UTC (permalink / raw) To: Andrew Stubbs; +Cc: gcc-patches 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 ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] openmp-simd-clone: Match shift type 2022-07-29 15:59 ` Jakub Jelinek @ 2022-07-29 17:03 ` Andrew Stubbs 2022-07-29 20:09 ` Jakub Jelinek 0 siblings, 1 reply; 4+ messages in thread From: Andrew Stubbs @ 2022-07-29 17:03 UTC (permalink / raw) To: Jakub Jelinek; +Cc: gcc-patches [-- Attachment #1: Type: text/plain, Size: 795 bytes --] 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 [-- Attachment #2: 220729-simd-clone-shift-types-2.patch --] [-- Type: text/plain, Size: 1281 bytes --] 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)), ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] openmp-simd-clone: Match shift type 2022-07-29 17:03 ` Andrew Stubbs @ 2022-07-29 20:09 ` Jakub Jelinek 0 siblings, 0 replies; 4+ messages in thread From: Jakub Jelinek @ 2022-07-29 20:09 UTC (permalink / raw) To: Andrew Stubbs; +Cc: gcc-patches 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 ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2022-07-29 20:09 UTC | newest] Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-07-29 15:53 [PATCH] openmp-simd-clone: Match shift type Andrew Stubbs 2022-07-29 15:59 ` Jakub Jelinek 2022-07-29 17:03 ` Andrew Stubbs 2022-07-29 20:09 ` Jakub Jelinek
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).