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
>> 

Reply via email to