Re: [RFC] [Patch]: Try and vectorize with shift for mult expr with power 2 integer constant.

[Jakub Jelinek <jakub@redhat.com>]

Hi!

> This is just an initial patch and tries to optimize integer type power 2
> constants.  I wanted to get feedback on this .  I bootstrapped and reg
> tested on aarch64-none-linux-gnu .

Thanks for working on it.
ChangeLog entry for the patch is missing, probably also some testcases.

> @@ -90,6 +94,7 @@ static vect_recog_func_ptr vect_vect_recog_func_ptrs[NUM_PATTERNS] = {
>  	vect_recog_rotate_pattern,
>  	vect_recog_vector_vector_shift_pattern,
>  	vect_recog_divmod_pattern,
> +        vect_recog_multconst_pattern,
>  	vect_recog_mixed_size_cond_pattern,
>  	vect_recog_bool_pattern};

Please watch formatting, the other lines are tab indented, so please use a
tab rather than 8 spaces.

> @@ -2147,6 +2152,87 @@ vect_recog_vector_vector_shift_pattern (vec<gimple> *stmts,
>    return pattern_stmt;
>  }
>  

Function comment is missing here.

> +static gimple
> +vect_recog_multconst_pattern (vec<gimple> *stmts,
> +                           tree *type_in, tree *type_out)

About the function name, wonder if just vect_recog_mult_pattern wouldn't be
enough.

> +  rhs_code = gimple_assign_rhs_code (last_stmt);
> +  switch (rhs_code)
> +    {
> +    case MULT_EXPR:
> +      break;
> +    default:
> +      return NULL;
> +    }

This looks too weird, I'd just do
  if (gimple_assign_rhs_code (last_stmt) != MULT_EXPR)
    return NULL;
(you handle just one pattern).

> +  /* If the target can handle vectorized multiplication natively,
> +     don't attempt to optimize this.  */
> +  optab = optab_for_tree_code (rhs_code, vectype, optab_default);

Supposedly you can use MULT_EXPR directly here.

> +  /* If target cannot handle vector left shift then we cannot 
> +     optimize and bail out.  */ 
> +  optab = optab_for_tree_code (LSHIFT_EXPR, vectype, optab_vector);
> +  if (!optab
> +      || optab_handler (optab, TYPE_MODE (vectype)) == CODE_FOR_nothing)
> +        return NULL;
> +
> +  if (integer_pow2p (oprnd1))
> +    {
> +      /* Pattern detected.  */
> +      if (dump_enabled_p ())
> +	dump_printf_loc (MSG_NOTE, vect_location,
> +			 "vect_recog_multconst_pattern: detected:\n");
> +
> +      tree shift;
> +      shift = build_int_cst (itype, tree_log2 (oprnd1));
> +      pattern_stmt = gimple_build_assign (vect_recog_temp_ssa_var (itype, NULL),
> +					  LSHIFT_EXPR, oprnd0, shift);
> +      if (dump_enabled_p ())
> +	dump_gimple_stmt_loc (MSG_NOTE, vect_location, TDF_SLIM, pattern_stmt,
> +                              0);
> +      stmts->safe_push (last_stmt);
> +      *type_in = vectype;
> +      *type_out = vectype;
> +      return pattern_stmt;
> +    } 

Trailing whitespace.
The integer_pow2p case (have you checked signed multiply by INT_MIN?)
is only one of the cases you can actually handle, you can look at
expand_mult for many other cases - e.g. multiplication by negated powers of
2, or call choose_mult_variant and handle whatever it returns.

	Jakub