Re: Unexpected interval comparison

Thank you for the comment.

At Mon, 03 Apr 2017 11:35:25 -0400, Tom Lane <tgl@sss.pgh.pa.us> wrote in <23053.1491233725@sss.pgh.pa.us>
> Kyotaro HORIGUCHI <horiguchi.kyotaro@lab.ntt.co.jp> writes:
> > Ok, the attached patch changes the result type of
> > interval_cmp_value from TimeOffset(=int64) to new 128 bit
> > LinearInterval. The value is hidden under the functions
> > interval_eq/ge.../cmp and all other stuff seems to use the
> > functions.
>
> Looking at this now ... why isn't the INT64_AU32 macro just
>
> #define INT64_AU32(i64) ((i64) >> 32)
>
> ?  The business with subtracting and re-adding 1 seems unnecessary, and it
> also creates a risk of overflow with the minimum possible int64 value.

It is equivalent to "i64 / (1<<32)" except for -INT64_MAX.

INT64_AU32 gives the value for the first term in the following
polynomial.

(int64)INT64_AU32(i64) * (2^32) + (int64)INT64_AL32(i64) = i64

The previous expression intended to avoid decimal arithmetic, but
gcc optimizes the simple division better (using cmovns-add-sar)
than the current INT64_AU32 (jmp-sar) so I changed it. This
doesn't suffer overflow.

-#define INT64_AU32(i64) (((i64) < 0 ? (((i64) - 1) >> 32) + 1: ((i64) >> 32)))
+#define INT64_AU32(i64) ((i64) / (1LL<<32))

In summation of terms in 128bit multiplication expression, I
noticed that the value of the second term's lower 32bit loses MSB
for certain cases. I changed LINEARINTERVAL_ADD_INT64 to accept
the MSB (as the 65th bit) separately.

The first attached is the revised patch and the second is
temporary sanity check code for non-128bit environment code. (but
works only on 128 bit environment)

regards,

--
Kyotaro Horiguchi
NTT Open Source Software Center