Re: [PATCH, rs6000] Early gimple folding of vec_mergeh and vec_mergel for float

[Richard Biener <richard.guenther@gmail.com>]

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