From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 104062 invoked by alias); 5 Aug 2015 08:51:16 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 104052 invoked by uid 89); 5 Aug 2015 08:51:15 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.3 required=5.0 tests=AWL,BAYES_05,FREEMAIL_FROM,RCVD_IN_DNSWL_LOW,SPF_PASS autolearn=ham version=3.3.2 X-HELO: mail-io0-f172.google.com Received: from mail-io0-f172.google.com (HELO mail-io0-f172.google.com) (209.85.223.172) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-GCM-SHA256 encrypted) ESMTPS; Wed, 05 Aug 2015 08:51:14 +0000 Received: by ioea135 with SMTP id a135so42611332ioe.1 for ; Wed, 05 Aug 2015 01:51:12 -0700 (PDT) MIME-Version: 1.0 X-Received: by 10.107.137.154 with SMTP id t26mr9220194ioi.13.1438764672397; Wed, 05 Aug 2015 01:51:12 -0700 (PDT) Received: by 10.107.32.140 with HTTP; Wed, 5 Aug 2015 01:51:12 -0700 (PDT) In-Reply-To: <7794A52CE4D579448B959EED7DD0A4723DD2081C@satlexdag06.amd.com> References: <7794A52CE4D579448B959EED7DD0A4723DD1F787@satlexdag06.amd.com> <20150728195340.GX1780@tucnak.redhat.com> <7794A52CE4D579448B959EED7DD0A4723DD201CC@satlexdag06.amd.com> <55BFAEE5.9010303@redhat.com> <7794A52CE4D579448B959EED7DD0A4723DD205E2@satlexdag06.amd.com> <7794A52CE4D579448B959EED7DD0A4723DD2081C@satlexdag06.amd.com> Date: Wed, 05 Aug 2015 08:51:00 -0000 Message-ID: Subject: Re: [RFC] [Patch]: Try and vectorize with shift for mult expr with power 2 integer constant. From: Richard Biener To: "Kumar, Venkataramanan" Cc: Jeff Law , Jakub Jelinek , "gcc-patches@gcc.gnu.org" Content-Type: text/plain; charset=UTF-8 X-IsSubscribed: yes X-SW-Source: 2015-08/txt/msg00241.txt.bz2 On Tue, Aug 4, 2015 at 6:49 PM, Kumar, Venkataramanan wrote: > Hi Richard, > > >> -----Original Message----- >> From: Richard Biener [mailto:richard.guenther@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 >> wrote: >> > Hi Jeff, >> > >> >> -----Original Message----- >> >> From: gcc-patches-owner@gcc.gnu.org [mailto:gcc-patches- >> >> owner@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.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. >> >> >> >> 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 >> >> > * 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 >> >> > * 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 > 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 ? The interesting case is of course LONG_MIN if you are vectorizing a long multiplication. Also check with arr[] being 'long', not 'unsigned long'. Note that with 'long' doing arr[i]*(-LONG_MIN) invokes undefined behavior. Richard. > > Thanks, >> Richard. >> >> >> >> >> >> >> >> >> > @@ -2147,6 +2152,140 @@ vect_recog_vector_vector_shift_pattern >> >> (vec *stmts, >> >> > return pattern_stmt; >> >> > } >> >> > >> >> > +/* Detect multiplication by constant which are postive or >> >> > +negatives of power 2, >> >> s/postive/positive/ >> >> >> >> >> >> Jeff >> > >> > Regards, >> > Venkat. >> >