Hi Richard, > -----Original Message----- > From: Richard Biener [mailto:richard.guent...@gmail.com] > Sent: Wednesday, August 05, 2015 5:11 PM > To: Kumar, Venkataramanan > Cc: Jeff Law; Jakub Jelinek; gcc-patches@gcc.gnu.org > Subject: Re: [RFC] [Patch]: Try and vectorize with shift for mult expr with > power 2 integer constant. > > On Tue, Aug 4, 2015 at 6:49 PM, Kumar, Venkataramanan > <venkataramanan.ku...@amd.com> wrote: > > Hi Richard, > > > > > >> -----Original Message----- > >> From: Richard Biener [mailto:richard.guent...@gmail.com] > >> Sent: Tuesday, August 04, 2015 4:07 PM > >> To: Kumar, Venkataramanan > >> Cc: Jeff Law; Jakub Jelinek; gcc-patches@gcc.gnu.org > >> Subject: Re: [RFC] [Patch]: Try and vectorize with shift for mult > >> expr with power 2 integer constant. > >> > >> On Tue, Aug 4, 2015 at 10:52 AM, Kumar, Venkataramanan > >> <venkataramanan.ku...@amd.com> wrote: > >> > Hi Jeff, > >> > > >> >> -----Original Message----- > >> >> From: gcc-patches-ow...@gcc.gnu.org [mailto:gcc-patches- > >> >> ow...@gcc.gnu.org] On Behalf Of Jeff Law > >> >> Sent: Monday, August 03, 2015 11:42 PM > >> >> To: Kumar, Venkataramanan; Jakub Jelinek > >> >> Cc: Richard Beiner (richard.guent...@gmail.com); > >> >> gcc-patches@gcc.gnu.org > >> >> Subject: Re: [RFC] [Patch]: Try and vectorize with shift for mult > >> >> expr with power 2 integer constant. > >> >> > >> >> On 08/02/2015 05:03 AM, Kumar, Venkataramanan wrote: > >> >> > Hi Jakub, > >> >> > > >> >> > Thank you for reviewing the patch. > >> >> > > >> >> > I have incorporated your comments in the attached patch. > >> >> Note Jakub is on PTO for the next 3 weeks. > >> > > >> > Thank you for this information. > >> > > >> >> > >> >> > >> >> > > >> >> > > >> >> > > >> >> > vectorize_mults_via_shift.diff.txt > >> >> > > >> >> > > >> >> > diff --git a/gcc/testsuite/gcc.dg/vect/vect-mult-patterns.c > >> >> > b/gcc/testsuite/gcc.dg/vect/vect-mult-patterns.c > >> >> Jakub would probably like more testcases :-) > >> >> > >> >> The most obvious thing to test would be other shift factors. > >> >> > >> >> A negative test to verify we don't try to turn a multiply by > >> >> non-constant or multiply by a constant that is not a power of 2 into > shifts. > >> > > >> > I have added negative test in the attached patch. > >> > > >> > > >> >> > >> >> [ Would it make sense, for example, to turn a multiply by 3 into a > >> >> shift-add sequence? As Jakub said, choose_mult_variant can be > >> >> your friend. ] > >> > > >> > Yes I will do that in a follow up patch. > >> > > >> > The new change log becomes > >> > > >> > gcc/ChangeLog > >> > 2015-08-04 Venkataramanan Kumar > >> <venkataramanan.ku...@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-04 Venkataramanan Kumar > >> <venkataramanan.ku...@amd.com> > >> > * gcc.dg/vect/vect-mult-pattern-1.c: New > >> > * gcc.dg/vect/vect-mult-pattern-2.c: New > >> > > >> > Bootstrapped and reg tested on aarch64-unknown-linux-gnu. > >> > > >> > Ok for trunk ? > >> > >> + if (TREE_CODE (oprnd0) != SSA_NAME > >> + || TREE_CODE (oprnd1) != INTEGER_CST > >> + || TREE_CODE (itype) != INTEGER_TYPE > >> > >> INTEGRAL_TYPE_P (itype) > >> > >> + optab = optab_for_tree_code (LSHIFT_EXPR, vectype, optab_vector); > >> + if (!optab > >> + || optab_handler (optab, TYPE_MODE (vectype)) == > >> CODE_FOR_nothing) > >> + return NULL; > >> + > >> > >> indent of the return stmt looks wrong > >> > >> + /* Handle constant operands that are postive or negative powers of 2. > >> + */ if ( wi::exact_log2 (oprnd1) != -1 || > >> + wi::exact_log2 (wi::neg (oprnd1)) != -1) > >> > >> no space after (, || goes to the next line. > >> > >> + { > >> + tree shift; > >> + > >> + if (wi::exact_log2 (oprnd1) != -1) > >> > >> please cache wi::exact_log2 > >> > >> in fact the first if () looks redundant if you simply put an else > >> return NULL after a else if (wi::exact_log2 (wi::neg (oprnd1)) != -1) > >> > >> Note that the issue with INT_MIN is that wi::neg (INT_MIN) is INT_MIN > >> again, but it seems that wi::exact_log2 returns -1 in that case so > >> you are fine (and in fact not handling this case). > >> > > > > I have updated your review comments in the attached patch. > > > > For the INT_MIN case, I am getting vectorized output with the patch. I > believe x86_64 also vectorizes but does not negates the results. > > > > #include <limits.h> > > unsigned long int __attribute__ ((aligned (64)))arr[100]; > > > > int i; > > #if 1 > > void test_vector_shifts() > > { > > for(i=0; i<=99;i++) > > arr[i]=arr[i] * INT_MIN; > > } > > #endif > > > > void test_vectorshift_via_mul() > > { > > for(i=0; i<=99;i++) > > arr[i]=arr[i]*(-INT_MIN); > > > > } > > > > Before > > --------- > > ldr x1, [x0] > > neg x1, x1, lsl 31 > > str x1, [x0], 8 > > cmp x0, x2 > > > > After > > ------- > > ldr q0, [x0] > > shl v0.2d, v0.2d, 31 > > neg v0.2d, v0.2d > > str q0, [x0], 16 > > cmp x1, x0 > > > > is this fine ? > > Btw, the patch is ok for trunk. It looks like it does the correct thing for > LONG_MIN. > > Thanks, > Richard.
Thanks you. I have committed the patch after a GCC bootstrap and regression testing on x86_64-unknown-linux-gnu machine. Ref: https://gcc.gnu.org/viewcvs/gcc?view=revision&revision=226675 > > > > Thanks, > >> Richard. > >> > >> >> > >> >> > >> >> > >> >> > @@ -2147,6 +2152,140 @@ vect_recog_vector_vector_shift_pattern > >> >> (vec<gimple> *stmts, > >> >> > return pattern_stmt; > >> >> > } > >> >> > > >> >> > +/* Detect multiplication by constant which are postive or > >> >> > +negatives of power 2, > >> >> s/postive/positive/ > >> >> > >> >> > >> >> Jeff > >> > > >> > Regards, > >> > Venkat. > >> > Regards, Venkat.
