Hi!
On Wed, Oct 10, 2018 at 10:14:01AM -0500, Will Schmidt wrote:
> 2018-10-09 Will Schmidt <will_schm...@vnet.ibm.com>
>
> * config/rs6000/rs6000.c: (fold_mergeeo_helper): New helper function.
* config/rs6000/rs6000.c (fold_mergeeo_helper): New helper function.
(i.e. no colon there)
> gimple *g = gimple_build_assign (lhs, VEC_PERM_EXPR, arg0, arg1, permute);
> gimple_set_location (g, gimple_location (stmt));
> gsi_replace (gsi, g, true);
> }
An empty line here please.
> +/* Helper function to handle the vector merge[eo] built-ins.
> + * The permute vector contains even or odd values that index
> + * across both arg1 and arg2. The even/odd-ness is handled via the
> + * shift argument passed in. */
And no leading stars. It isn't clear from this what value should be in
the permute vector or the shift variable. Maybe you shouldn't mention
the vector here at all? And use a "bool odd" parameter (or "use_odd" like
for mergehl"?
> + /* 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 (types_compatible_p (TREE_TYPE (lhs_type),
> + TREE_TYPE (V2DF_type_node)))
> + permute_type = V2DI_type_node;
> + else if (types_compatible_p (TREE_TYPE (lhs_type),
> + TREE_TYPE (V4SF_type_node)))
> + permute_type = V4SI_type_node;
> + else
> + gcc_unreachable ();
> + }
Maybe this stuff should be factored out? mergehl has similar.
> + /* Build the permute vector. */
> + for (int i = 0; i < n_elts / 2; i++)
> + {
> + elts.safe_push (build_int_cst (TREE_TYPE (permute_type),
> + 2*i + shift));
> + elts.safe_push (build_int_cst (TREE_TYPE (permute_type),
> + 2*i + shift + n_elts));
> + }
All the indents are wrong here :-)
/* Build the permute vector. */
for (int i = 0; i < n_elts / 2; i++)
{
elts.safe_push (build_int_cst (TREE_TYPE (permute_type),
2*i + shift));
elts.safe_push (build_int_cst (TREE_TYPE (permute_type),
2*i + shift + n_elts));
}
> + case P8V_BUILTIN_VMRGEW_V4SI:
> + case P8V_BUILTIN_VMRGEW_V2DI:
> + case P8V_BUILTIN_VMRGEW_V4SF:
> + case P8V_BUILTIN_VMRGEW_V2DF:
> + fold_mergeeo_helper (gsi, stmt, 0);
> + return true;
The "fold" line should be indented only 6 chars deep, too (it's wrong in
the existing code as well).
Okay for trunk with those things improved. Thanks!
Segher
