On August 17, 2018 5:02:53 PM GMT+02:00, Will Schmidt <will_schm...@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 >> <seg...@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 >>