On Tue, 20 Jun 2017, James Greenhalgh wrote:
>
> On Fri, Jun 16, 2017 at 11:41:57AM +0200, Richard Biener wrote:
> > On Fri, 16 Jun 2017, James Greenhalgh wrote:
> > > On Mon, Jun 12, 2017 at 03:56:25PM +0200, Richard Biener wrote:
> > > > + 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.
>
> Thanks, I've made that change.
>
> > > 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.
>
> OK. I've added "target_has_vector_rshift_p" for this purpose.
Actually I was looking for a bit more generic
bool
target_supports_op_p (tree type, enum tree_code code,
enum optab_subtype = optab_default)
{
optab ot = optab_for_tree_code (code, type, optab_subtype);
return (ot != unknown_optab
&& optab_handler (ot, TYPE_MODE (type)) != CODE_FOR_nothing);
}
and you using target_supports_op_p (type, RSHIFT_EXPR, optab_scalar)
|| target_supports_op_p (type, RSHIFT_EXPR, optab_vector)
Ok with that change.
Thanks,
Richard.
> Bootstrapped and tested on aarch64-none-linux-gnu with no issues.
>
> OK?
>
> Thanks,
> James
>
> ---
> gcc/
>
> 2017-06-19 James Greenhalgh <[email protected]>
>
> * match.pd (A / (1 << B) -> A >> B): New.
> * generic-match-head.c: Include optabs-tree.h.
> * gimple-match-head.c: Likewise.
> * optabs-tree.h (target_has_vector_rshift_p): New.
> * optabs-tree.c (target_has_vector_rshift_p): New.
>
> gcc/testsuite/
>
> 2017-06-19 James Greenhalgh <[email protected]>
>
> * gcc.dg/tree-ssa/forwprop-37.c: New.
>
>
--
Richard Biener <[email protected]>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB
21284 (AG Nuernberg)
