On Fri, Nov 18, 2016 at 03:31:40AM -0600, Segher Boessenkool wrote:
> Hi Dominik,
> 
> On Thu, Nov 17, 2016 at 04:53:47PM +0100, Dominik Vogt wrote:
> > +/* A convenience macro to determine whether a SIZE lies inclusively
> > +   within [1, RANGE], POS lies inclusively within between
> > +   [0, RANGE - 1] and the sum lies inclusively within [1, RANGE].  */
> > +#define SIZE_POS_IN_RANGE(SIZE, POS, RANGE) \
> > +  (IN_RANGE (POS, 0, (RANGE) - 1) \
> > +   && IN_RANGE (SIZE, 1, RANGE) \
> > +   && IN_RANGE ((SIZE) + (POS), 1, RANGE))
> 
> You should put parens around every use of SIZE, POS, RANGE -- there could
> be a comma operator in the macro argument.

Good point.  That wouldn't matter for a backend macro, but in
system.h it does.

> You don't check if SIZE+POS overflows / wraps around.  If you really don't
> care about that, you can lose the
> 
> > +   && IN_RANGE (SIZE, 1, RANGE) \
> 
> part as well?

The macro is _intended_ for checking the (possibly bogus)
arguments of zero_extract and sing_extract that combine may
generate.  SIZE is some unsigned value, but POS might be unsigned
or signed, and actually have a negative value (INTVAL
(operands[x]) or UINTVAL (operands[x]))), and RANGE is typically
64 or another small value.

Anyway, the macro should work for any inputs (except RANGE <= 0).

How about this:

--
/* A convenience macro to determine whether a SIZE lies inclusively
   within [1, UPPER], POS lies inclusively within between
   [0, UPPER - 1] and the sum lies inclusively within [1, UPPER].
   UPPER must be >= 1.  */
#define SIZE_POS_IN_RANGE(SIZE, POS, UPPER) \
  (IN_RANGE ((POS), 0, (unsigned HOST_WIDE_INT) (UPPER) - 1) \
   && IN_RANGE ((SIZE), 1, (unsigned HOST_WIDE_INT) (UPPER) \
                           - (unsigned HOST_WIDE_INT)(POS)))
--

IN_RANGE(POS...) makes sure that POS is a non-negative number
smaller than UPPER, so (UPPER) - (POS) does not wrap.  Or is there
some case that the new macro does not handle?

Ciao

Dominik ^_^  ^_^

-- 

Dominik Vogt
IBM Germany

Reply via email to