From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 45732 invoked by alias); 16 Jun 2017 09:41:57 -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 45721 invoked by uid 89); 16 Jun 2017 09:41:56 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-6.9 required=5.0 tests=BAYES_00,GIT_PATCH_2,SPF_PASS,T_RP_MATCHES_RCVD autolearn=ham version=3.3.2 spammy=ok! X-HELO: mx1.suse.de Received: from mx2.suse.de (HELO mx1.suse.de) (195.135.220.15) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 16 Jun 2017 09:41:55 +0000 Received: from relay1.suse.de (charybdis-ext.suse.de [195.135.220.254]) by mx1.suse.de (Postfix) with ESMTP id DB1DEABF4; Fri, 16 Jun 2017 09:41:57 +0000 (UTC) Date: Fri, 16 Jun 2017 09:41:00 -0000 From: Richard Biener To: James Greenhalgh cc: gcc-patches@gcc.gnu.org, nd@arm.com Subject: Re: [Patch match.pd] Fold (A / (1 << B)) to (A >> B) In-Reply-To: <1497604990-5082-1-git-send-email-james.greenhalgh@arm.com> Message-ID: References: <1497273586-9046-1-git-send-email-james.greenhalgh@arm.com> <1497604990-5082-1-git-send-email-james.greenhalgh@arm.com> User-Agent: Alpine 2.20 (LSU 67 2015-01-07) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII X-SW-Source: 2017-06/txt/msg01175.txt.bz2 On Fri, 16 Jun 2017, James Greenhalgh wrote: > > On Mon, Jun 12, 2017 at 03:56:25PM +0200, Richard Biener wrote: > > On Mon, 12 Jun 2017, James Greenhalgh wrote: > > > > > > > > Hi, > > > > > > As subject, for the testcase in the patch: > > > > > > unsigned long > > > f2 (unsigned long a, int b) > > > { > > > unsigned long x = 1UL << b; > > > return a / x; > > > } > > > > > > We currently generate: > > > > > > f2: > > > mov x2, 1 > > > lsl x1, x2, x1 > > > udiv x0, x0, x1 > > > ret > > > > > > Which could instead be transformed to: > > > > > > f2: > > > lsr x0, x0, x1 > > > ret > > > > > > OK? > > > > + We can't do the same for signed A, as it might be negative, which > > would > > + introduce undefined behaviour. */ > > > > huh, AFAIR it is _left_ shift of negative values that invokes > > undefined behavior. > > You're right this is not a clear comment. The problem is not undefined > behaviour, so that text needs to go, but rounding towards/away from zero > for signed negative values. Division will round towards zero, arithmetic > right shift away from zero. For example in: > > -1 / (1 << 1) != -1 >> 1 > = -1 / 2 > = 0 = -1 > > I've rewritten the comment to make it clear this is why we can only make > this optimisation for unsigned values. Ah, of course. You could use if ((TYPE_UNSIGNED (type) || tree_expr_nonnegative_p (@0)) here as improvement. > See, for example, gcc.c-torture/execute/pr34070-2.c > > > Note that as you are accepting vectors you need to make sure the > > target actually supports arithmetic right shift of vectors > > (you only know it supports left shift and division -- so it might > > be sort-of-superfluous to check in case there is no arch that supports > > those but not the other). > > I've added a check for that using optabs, is that the right way to do this? + && (!VECTOR_TYPE_P (type) + || optab_for_tree_code (RSHIFT_EXPR, type, optab_vector) + || optab_for_tree_code (RSHIFT_EXPR, type, optab_scalar))) is not enough -- you need sth like optab ot = optab_for_tree_code (RSHIFT_EXPR, type, optab_vector); if (ot != unknown_optab && optab_handler (ot, TYPE_MODE (type)) != CODE_FOR_nothing) .. ok! ... ideally we'd have a helper for this in optab-tree.[ch], tree-vect-patterns.c could also make use of that. Thanks, Richard. > Bootstrapped and tested on aarch64-none-linux-gnu with no issues. > > OK? > > Thanks, > James > > --- > gcc/ > > 2017-06-13 James Greenhalgh > > * match.pd (A / (1 << B) -> A >> B): New. > * generic-match-head.c: Include optabs-tree.h. > * gimple-match-head.c: Likewise. > > gcc/testsuite/ > > 2017-06-13 James Greenhalgh > > * gcc.dg/tree-ssa/forwprop-37.c: New. > > -- Richard Biener SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)