RFR (XS): 8191915: JCK tests produce incorrect results with C2

[Rickard Bäckman]

Yes you are right, it doesn't work for all values. Seems to work for
negatives though?

However casting the result back to signed after doing the multiplication
and doing the division with the signed values get us back to getting the
overflow check correct and no undefined behaviour as far as I can tell?

/R

On 12/14, Tobias Hartmann wrote:
> Hi Rickard,
> 
> I don't understand why casting from signed to unsigned is correct here because if there's an overflow of the signed
> range, there is not necessarily and overflow of the unsigned range, right?
> 
> For example, this program:
> 
>   signed long long sv1 = LONG_MAX/2 + 1;
>   signed long long sv2 = 2;
>   signed long long sR = sv1 * sv2;
>   printf("%li\n", sR);
> 
>   unsigned long long uv1 = (unsigned long long) sv1;	
>   unsigned long long uv2 = (unsigned long long) sv2;
>   unsigned long long uR = uv1 * uv2;
>   printf("%lu\n", uR);
> 
>   if (uR / uv1 != uv2) {
>     printf("Overflow detected");
>   } else {
>     printf("No overflow detected");
>   }
> 
> Output:
> 
>   -9223372036854775808
>   9223372036854775808
>   No overflow detected
> 
> So there is an overflow of the signed long but the unsigned long does not overflow and thus an overflow is not detected.
> Do I miss something?
> 
> Regarding the jtreg test: Please do not use bug numbers as test names but use something more meaningful like
> TestLongMulOverflow or similar. I would also suggest to move the allocation of the test instance out of the loop and
> check if 500.000 iterations are really necessary to trigger compilation (you can run with -Xbatch and maybe
> -XX:-TieredCompilation).
> 
> Best regards,
> Tobias
> 
> On 14.12.2017 08:30, Rickard Bäckman wrote:
> > Hi, can I please have this change reviewed?
> > 
> > Summary is that we had an overflow check for mulitplyExact(long, long)
> > intrinsic that relied on undefined behaviour. clang / llvm started
> > optimizing that so that if a = b * c then a / b must be equal to c and
> > removed half of the overflow check. 
> > 
> > Changing the multiplication and division to be unsigned.
> > 
> > Webrev: http://cr.openjdk.java.net/~rbackman/8191915/
> > Bug: https://bugs.openjdk.java.net/browse/JDK-8191915
> > 
> > Thanks
> > /R
> >