On Mon, Jun 23, 2014 at 04:50:49PM +0800, Thomas Preud'homme wrote:
> > Sent: Monday, June 23, 2014 4:37 PM
> >
> > On Mon, Jun 23, 2014 at 10:18:16AM +0200, Richard Biener wrote:
> > > > --- a/gcc/tree-ssa-math-opts.c
> > > > +++ b/gcc/tree-ssa-math-opts.c
> > > > @@ -1741,6 +1741,8 @@ find_bswap_1 (gimple stmt, struct
> > symbolic_number *n, int limit)
> > > > if (n->size % BITS_PER_UNIT != 0)
> > > > return NULL_TREE;
> > > > n->size /= BITS_PER_UNIT;
> > > > + if (n->size > (int)sizeof (unsigned HOST_WIDEST_INT))
> > > > + return NULL_TREE;
> >
> > This looks wrong, while the bswap pass is guarded with BITS_PER_UNIT == 8
> > check (i.e. target), you don't know of HOST_BITS_PER_CHAR is 8.
> > I'd move the test before the division by BITS_PER_UNIT, and compare
> > against HOST_BITS_PER_WIDEST_INT.
>
> I may misunderstand you but I don't think there is a problem here because we
> just check if we can create a value on the host with as many bytes as the
> value
> on the target. The value on the host is different, with each byte being a
> number from 1 to SIZE, SIZE being the number of bytes on the target. So this
> would fail only if the target value has so many bytes that this number of byte
> cannot be represented in a HOST_WIDEST_INT.
Host could e.g. in theory have CHAR_BIT 32, while target BITS_PER_UNIT 8
(otherwise bswap pass would give up). sizeof (unsigned HOST_WIDE_INT) could
very well be 2 in that case.
Anyway, another option is to also not run the bswap pass if CHAR_BIT != 8,
e.g. fold-const.c does something similar when deciding if VIEW_CONVERT_EXPR
of constants can be safely folded.
> > BTW, the formatting is wrong too, the (int) cast should be followed by
> > space.
>
> Right, but note that I merely followed the current style in this file. There
> are
> many more occurences of this style mistake in this file. Do you want me to
> fix this one anyway?
Just don't introduce new formatting issues and on lines you touch anyway
also fix formatting issues.
Jakub
