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