[Patch match.pd] Fold (A / (1 << B)) to (A >> B)
Richard Biener
rguenther@suse.de
Fri Jun 16 09:41:00 GMT 2017
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 <james.greenhalgh@arm.com>
>
> * 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 <james.greenhalgh@arm.com>
>
> * gcc.dg/tree-ssa/forwprop-37.c: New.
>
>
--
Richard Biener <rguenther@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)
More information about the Gcc-patches
mailing list