RFR: 8309266: C2: assert(final_con == (jlong)final_int) failed: final value should be integer

[Emanuel Peter]

On Thu, 8 Jun 2023 07:35:52 GMT, Daohan Qu <duke at openjdk.org> wrote:

>> @quadhier Thanks for looking into this! This but is currently not assigned to you. Please always make sure that you have it assigned to you, or at least mention in JIRA that you are working on it. Currently, @enothum had it assigned and was also working on it.
>> 
>> The regression test cleanly reproduces before the patch, good.
>> 
>> Why is the overflow acceptable? Does that not mean that the calculation did something wrong?
>> 
>> In the example, we have
>> 
>> init_con = 3
>> limit_con = 2147483647 = max_jint
>> stride_con = 3
>> stride_m = stride_con - 1 = 2
>> trip_count = (limit_con - init_con + stride_m)/stride_con = 715827882
>> final_con = init_con + stride_con*trip_count = 2147483649 = max_jint + 2 (overflow!)
>> final_int = -2147483647 (overflow!)
>> 
>> 
>> Does that not mean that we mis-calculated the `trip_count`? If it was 1 less, we would not have an overflow. Would that not fix the issue in a simpler way? Or did I get something wrong?
>> 
>> Let's expand the formula:
>> 
>> final_con = init_con + stride_con*trip_count
>> final_con = init_con + stride_con * ((limit_con - init_con + stride_m) / stride_con)
>> final_con = init_con + stride_con * ((limit_con - init_con + stride_con - 1) / stride_con)
>> 
>> 
>> Is the issue not that instead of coming up with a final value that is slightly below `limit_con`, we come up with one that is slightly above `limit_con`, and can thus overflow?
>> 
>> Would this be correct instead (for positive stride)?
>> 
>> final_con = init_con + stride_con * ((limit_con - init_con + 1 - stride_con) / stride_con)
>> 
>> 
>> Could the limit type ever overflow at runtime? Does the loop limit check not prevent that (unsure)?
>> 
>> Can you explain again why exactly we calculate what we calculate here, and why that is correct?
>
> This patch causes many compilation timeouts. I'd close this PR and wait for a better fix. Thanks for your review and suggestion @eme64 !

@quadhier So you intend to keep working on this?

-------------

PR Comment: https://git.openjdk.org/jdk/pull/14353#issuecomment-1582063718