| Message ID | 1533669898.4452.11.camel@brimstone.rchland.ibm.com |
|---|---|
| State | New |
| Headers | show |
| Series | [rs6000] Early gimple folding of vec_mergeh and vec_mergel for float | expand |
Hi! On Tue, Aug 07, 2018 at 02:24:58PM -0500, Will Schmidt wrote: > This adds support for gimple folding of vec_mergeh and vec_mergel > for float and double types. Support for the integral types is already > in-tree. > + /* The permute_type will match the lhs for integral types. For double and > + float types, the permute type needs to map to the V2 or V4 type that > + matches size. */ > + tree permute_type; > + if (INTEGRAL_TYPE_P (TREE_TYPE (lhs_type))) > + permute_type = lhs_type; > + else > + if (TREE_TYPE (lhs_type) == TREE_TYPE (V2DF_type_node)) > + permute_type = V2DI_type_node; > + else if (TREE_TYPE (lhs_type) == TREE_TYPE (V4SF_type_node)) > + permute_type = V4SI_type_node; > + else > + gcc_unreachable (); Please write this as if (INTEGRAL_TYPE_P (TREE_TYPE (lhs_type))) permute_type = lhs_type; else if (TREE_TYPE (lhs_type) == TREE_TYPE (V2DF_type_node)) permute_type = V2DI_type_node; else if (TREE_TYPE (lhs_type) == TREE_TYPE (V4SF_type_node)) permute_type = V4SI_type_node; else gcc_unreachable (); or, if you want to emphasize integer vs. float: if (INTEGRAL_TYPE_P (TREE_TYPE (lhs_type))) permute_type = lhs_type; else { if (TREE_TYPE (lhs_type) == TREE_TYPE (V2DF_type_node)) permute_type = V2DI_type_node; else if (TREE_TYPE (lhs_type) == TREE_TYPE (V4SF_type_node)) permute_type = V4SI_type_node; else gcc_unreachable (); } Okay for trunk with that changed. Thanks! Segher
On Wed, Aug 8, 2018 at 12:59 AM Segher Boessenkool <segher@kernel.crashing.org> wrote: > > Hi! > > On Tue, Aug 07, 2018 at 02:24:58PM -0500, Will Schmidt wrote: > > This adds support for gimple folding of vec_mergeh and vec_mergel > > for float and double types. Support for the integral types is already > > in-tree. > > > + /* The permute_type will match the lhs for integral types. For double and > > + float types, the permute type needs to map to the V2 or V4 type that > > + matches size. */ > > + tree permute_type; > > + if (INTEGRAL_TYPE_P (TREE_TYPE (lhs_type))) > > + permute_type = lhs_type; > > + else > > + if (TREE_TYPE (lhs_type) == TREE_TYPE (V2DF_type_node)) > > + permute_type = V2DI_type_node; > > + else if (TREE_TYPE (lhs_type) == TREE_TYPE (V4SF_type_node)) > > + permute_type = V4SI_type_node; > > + else > > + gcc_unreachable (); > > Please write this as > > if (INTEGRAL_TYPE_P (TREE_TYPE (lhs_type))) > permute_type = lhs_type; > else if (TREE_TYPE (lhs_type) == TREE_TYPE (V2DF_type_node)) > permute_type = V2DI_type_node; > else if (TREE_TYPE (lhs_type) == TREE_TYPE (V4SF_type_node)) > permute_type = V4SI_type_node; > else > gcc_unreachable (); > > or, if you want to emphasize integer vs. float: > > if (INTEGRAL_TYPE_P (TREE_TYPE (lhs_type))) > permute_type = lhs_type; > else > { > if (TREE_TYPE (lhs_type) == TREE_TYPE (V2DF_type_node)) Are you sure lhs_type is never qualified? That is, for a GIMPLE folder I'd have expected if (types_compatible_p (TREE_TYPE (lhs_type), TREE_TYPE (V2DF_type_node))) for GENERIC if (TYPE_MAIN_VARIANT (TREE_TYPE (lhs_type)) == TREE_TYPE (V2DF_type_node)) Richard. > permute_type = V2DI_type_node; > else if (TREE_TYPE (lhs_type) == TREE_TYPE (V4SF_type_node)) > permute_type = V4SI_type_node; > else > gcc_unreachable (); > } > > Okay for trunk with that changed. Thanks! > > > Segher
On Fri, 2018-08-17 at 16:00 +0200, Richard Biener wrote: > On Wed, Aug 8, 2018 at 12:59 AM Segher Boessenkool > <segher@kernel.crashing.org> wrote: > > > > Hi! > > > > On Tue, Aug 07, 2018 at 02:24:58PM -0500, Will Schmidt wrote: > > > This adds support for gimple folding of vec_mergeh and vec_mergel > > > for float and double types. Support for the integral types is already > > > in-tree. > > > > > + /* The permute_type will match the lhs for integral types. For double and > > > + float types, the permute type needs to map to the V2 or V4 type that > > > + matches size. */ > > > + tree permute_type; > > > + if (INTEGRAL_TYPE_P (TREE_TYPE (lhs_type))) > > > + permute_type = lhs_type; > > > + else > > > + if (TREE_TYPE (lhs_type) == TREE_TYPE (V2DF_type_node)) > > > + permute_type = V2DI_type_node; > > > + else if (TREE_TYPE (lhs_type) == TREE_TYPE (V4SF_type_node)) > > > + permute_type = V4SI_type_node; > > > + else > > > + gcc_unreachable (); > > > > Please write this as > > > > if (INTEGRAL_TYPE_P (TREE_TYPE (lhs_type))) > > permute_type = lhs_type; > > else if (TREE_TYPE (lhs_type) == TREE_TYPE (V2DF_type_node)) > > permute_type = V2DI_type_node; > > else if (TREE_TYPE (lhs_type) == TREE_TYPE (V4SF_type_node)) > > permute_type = V4SI_type_node; > > else > > gcc_unreachable (); > > > > or, if you want to emphasize integer vs. float: > > > > if (INTEGRAL_TYPE_P (TREE_TYPE (lhs_type))) > > permute_type = lhs_type; > > else > > { > > if (TREE_TYPE (lhs_type) == TREE_TYPE (V2DF_type_node)) > > Are you sure lhs_type is never qualified? For the V2DF and V4SF types, I am mostly sure, but since you bring it up I will admit I am not positive. :-) > That is, for a GIMPLE folder > I'd have expected > > if (types_compatible_p (TREE_TYPE (lhs_type), TREE_TYPE (V2DF_type_node))) > for GENERIC > > if (TYPE_MAIN_VARIANT (TREE_TYPE (lhs_type)) == TREE_TYPE (V2DF_type_node)) Either/both of those seem more robust than what I had come up with, I'll plan to make an update here. Any guidance on whether I should prefer "types_compatible_p" versus the GENERIC "TYPE_MAIN_VARIANT" ? I see more references to the latter in rs6000.c , but the former seems to make better sense to me just by its name. Thanks for the review, -Will > Richard. > > > permute_type = V2DI_type_node; > > else if (TREE_TYPE (lhs_type) == TREE_TYPE (V4SF_type_node)) > > permute_type = V4SI_type_node; > > else > > gcc_unreachable (); > > } > > > > Okay for trunk with that changed. Thanks! > > > > > > Segher >
On August 17, 2018 5:02:53 PM GMT+02:00, Will Schmidt <will_schmidt@vnet.ibm.com> wrote: >On Fri, 2018-08-17 at 16:00 +0200, Richard Biener wrote: >> On Wed, Aug 8, 2018 at 12:59 AM Segher Boessenkool >> <segher@kernel.crashing.org> wrote: >> > >> > Hi! >> > >> > On Tue, Aug 07, 2018 at 02:24:58PM -0500, Will Schmidt wrote: >> > > This adds support for gimple folding of vec_mergeh and >vec_mergel >> > > for float and double types. Support for the integral types is >already >> > > in-tree. >> > >> > > + /* The permute_type will match the lhs for integral types. >For double and >> > > + float types, the permute type needs to map to the V2 or V4 >type that >> > > + matches size. */ >> > > + tree permute_type; >> > > + if (INTEGRAL_TYPE_P (TREE_TYPE (lhs_type))) >> > > + permute_type = lhs_type; >> > > + else >> > > + if (TREE_TYPE (lhs_type) == TREE_TYPE (V2DF_type_node)) >> > > + permute_type = V2DI_type_node; >> > > + else if (TREE_TYPE (lhs_type) == TREE_TYPE (V4SF_type_node)) >> > > + permute_type = V4SI_type_node; >> > > + else >> > > + gcc_unreachable (); >> > >> > Please write this as >> > >> > if (INTEGRAL_TYPE_P (TREE_TYPE (lhs_type))) >> > permute_type = lhs_type; >> > else if (TREE_TYPE (lhs_type) == TREE_TYPE (V2DF_type_node)) >> > permute_type = V2DI_type_node; >> > else if (TREE_TYPE (lhs_type) == TREE_TYPE (V4SF_type_node)) >> > permute_type = V4SI_type_node; >> > else >> > gcc_unreachable (); >> > >> > or, if you want to emphasize integer vs. float: >> > >> > if (INTEGRAL_TYPE_P (TREE_TYPE (lhs_type))) >> > permute_type = lhs_type; >> > else >> > { >> > if (TREE_TYPE (lhs_type) == TREE_TYPE (V2DF_type_node)) >> >> Are you sure lhs_type is never qualified? > >For the V2DF and V4SF types, I am mostly sure, but since you bring it >up >I will admit I am not positive. :-) > >> That is, for a GIMPLE folder >> I'd have expected >> >> if (types_compatible_p (TREE_TYPE (lhs_type), TREE_TYPE >(V2DF_type_node))) > >> for GENERIC >> >> if (TYPE_MAIN_VARIANT (TREE_TYPE (lhs_type)) == TREE_TYPE >(V2DF_type_node)) > >Either/both of those seem more robust than what I had come up with, >I'll >plan to make an update here. > >Any guidance on whether I should prefer "types_compatible_p" versus the >GENERIC "TYPE_MAIN_VARIANT" ? I see more references to the latter in >rs6000.c , but the former seems to make better sense to me just by its >name. The former if this is called from the GIMPLE stmts folding hook. Richard. >Thanks for the review, >-Will > > >> Richard. >> >> > permute_type = V2DI_type_node; >> > else if (TREE_TYPE (lhs_type) == TREE_TYPE (V4SF_type_node)) >> > permute_type = V4SI_type_node; >> > else >> > gcc_unreachable (); >> > } >> > >> > Okay for trunk with that changed. Thanks! >> > >> > >> > Segher >>
diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c index 6cb5c87..35c32be 100644 --- a/gcc/config/rs6000/rs6000.c +++ b/gcc/config/rs6000/rs6000.c @@ -15119,24 +15119,39 @@ fold_mergehl_helper (gimple_stmt_iterator *gsi, gimple *stmt, int use_high) { tree arg0 = gimple_call_arg (stmt, 0); tree arg1 = gimple_call_arg (stmt, 1); tree lhs = gimple_call_lhs (stmt); tree lhs_type = TREE_TYPE (lhs); - tree lhs_type_type = TREE_TYPE (lhs_type); int n_elts = TYPE_VECTOR_SUBPARTS (lhs_type); int midpoint = n_elts / 2; int offset = 0; if (use_high == 1) offset = midpoint; - tree_vector_builder elts (lhs_type, VECTOR_CST_NELTS (arg0), 1); + /* The permute_type will match the lhs for integral types. For double and + float types, the permute type needs to map to the V2 or V4 type that + matches size. */ + tree permute_type; + if (INTEGRAL_TYPE_P (TREE_TYPE (lhs_type))) + permute_type = lhs_type; + else + if (TREE_TYPE (lhs_type) == TREE_TYPE (V2DF_type_node)) + permute_type = V2DI_type_node; + else if (TREE_TYPE (lhs_type) == TREE_TYPE (V4SF_type_node)) + permute_type = V4SI_type_node; + else + gcc_unreachable (); + + tree_vector_builder elts (permute_type, VECTOR_CST_NELTS (arg0), 1); for (int i = 0; i < midpoint; i++) { - elts.safe_push (build_int_cst (lhs_type_type, offset + i)); - elts.safe_push (build_int_cst (lhs_type_type, offset + n_elts + i)); + elts.safe_push (build_int_cst (TREE_TYPE (permute_type), + offset + i)); + elts.safe_push (build_int_cst (TREE_TYPE (permute_type), + offset + n_elts + i)); } tree permute = elts.build (); gimple *g = gimple_build_assign (lhs, VEC_PERM_EXPR, arg0, arg1, permute); @@ -15757,18 +15772,22 @@ rs6000_gimple_fold_builtin (gimple_stmt_iterator *gsi) case ALTIVEC_BUILTIN_VMRGLH: case ALTIVEC_BUILTIN_VMRGLW: case VSX_BUILTIN_XXMRGLW_4SI: case ALTIVEC_BUILTIN_VMRGLB: case VSX_BUILTIN_VEC_MERGEL_V2DI: + case VSX_BUILTIN_XXMRGLW_4SF: + case VSX_BUILTIN_VEC_MERGEL_V2DF: fold_mergehl_helper (gsi, stmt, 1); return true; /* vec_mergeh (integrals). */ case ALTIVEC_BUILTIN_VMRGHH: case ALTIVEC_BUILTIN_VMRGHW: case VSX_BUILTIN_XXMRGHW_4SI: case ALTIVEC_BUILTIN_VMRGHB: case VSX_BUILTIN_VEC_MERGEH_V2DI: + case VSX_BUILTIN_XXMRGHW_4SF: + case VSX_BUILTIN_VEC_MERGEH_V2DF: fold_mergehl_helper (gsi, stmt, 0); return true; /* d = vec_pack (a, b) */ case P8V_BUILTIN_VPKUDUM:
Hi, This adds support for gimple folding of vec_mergeh and vec_mergel for float and double types. Support for the integral types is already in-tree. This change includes updates to the fold_mergehl_helper function to handle creating the (integral) permute vector when the vector type non integer. Relevant testcases are already in-tree. OK for trunk? Thanks, -Will [gcc] 2018-08-07 Will Schmidt <will_schmidt@vnet.ibm.com> * config/rs6000/rs600.c (rs6000_gimple_fold_builtin): Add entries to allow folding of mergeh() and mergel() for the float and double types. (fold_mergehl_helper): Rework to handle building a permute tree for float vectors.