Hi Jakub, 

Thank you for reviewing the patch.

I have incorporated your comments in the attached patch.


> -----Original Message-----
> From: Jakub Jelinek [mailto:jakub@redhat.com]
> Sent: Wednesday, July 29, 2015 1:24 AM
> To: Kumar, Venkataramanan
> Cc: Richard Beiner (richard.guenther@gmail.com); gcc-patches@gcc.gnu.org
> Subject: Re: [RFC] [Patch]: Try and vectorize with shift for mult expr with
> power 2 integer constant.
> 
> 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.

I have added patterns that detect mults with power of 2 constants and convert them to shifts in this patch.
I will do a follow up patch for other variants as done in "expand_mult" and "choose_mult_variant".

INT_MIN case, I have not yet checked. 

 I thought of adding like this. 
if (wi::eq_p (oprnd1, HOST_WIDE_INT_MIN))
    return NULL;

But wanted to confirm with you since I don't understand  this clearly.  Is this because Var * INT_MIN !=  Var << 31  ?

 I tried this case for "int" or long int type arr  

void test_vector_shifts()
{
        for(i=0; i<=99;i++)
        arr[i]=arr[i]*INT_MIN;
}

Int type-  Looks like  without my patch aarch64 vectorizes it using shifts.
        ldr     q0, [x0]
        shl     v0.4s, v0.4s, 31
        str     q0, [x0], 16

For long int  type -  without my patch it converts to shifts but vectorization did not happen.
        ldr     x1, [x0]
        neg     x1, x1, lsl 31
        str     x1, [x0], 8

> 
> 	Jakub

gcc/ChangeLog
2015-08-02  Venkataramanan Kumar  <Venkataramanan.kumar@amd.com>
     * tree-vect-patterns.c (vect_recog_mult_pattern): New function for vectorizing
        multiplication patterns.
     * tree-vectorizer.h: Adjust the number of patterns.

gcc/testsuite/ChangeLog
2015-08-02  Venkataramanan Kumar  <Venkataramanan.kumar@amd.com>
     * gcc.dg/vect/vect-mult-patterns.c: New
 

Regards,
Venkat.