RFR (M) 8042786: Proper fix for 8032566

[Vladimir Kozlov]

Thank you, Igor

Vladimir

On 5/14/14 7:13 PM, Igor Veresov wrote:
> Looks good.
>
> igor
>
> On May 14, 2014, at 5:26 PM, Vladimir Kozlov <vladimir.kozlov at oracle.com> wrote:
>
>> http://cr.openjdk.java.net/~kvn/8042786/webrev/
>> https://bugs.openjdk.java.net/browse/JDK-8042786
>>
>> 8032566 was fixed in jdk8 by disabling autobox elimination: -XX:-EliminateAutoBox.
>>
>> This fix will undo that change and fix the problem. In jdk9 8032566 change was not applied before, I will push it first so that I could use the same 8042786 changeset for 8u backport.
>>
>> The problem was that range check for loading from boxing cache was not eliminated but the load was removed when integer value is not in the cached range and it includes maxint: [256,maxint].
>>
>> When autobox elimination is enabled the method IfNode::fold_compares() converts the value check in Integer.valueOf() to array range check and remove it as duplicate of the range check generated for the load:
>>
>>     public static Integer valueOf(int i) {
>>         if (i >= IntegerCache.low && i <= IntegerCache.high)
>>             return IntegerCache.cache[i + (-IntegerCache.low)];
>>         return new Integer(i);
>>     }
>>
>> The problem rise when result of AddI may overflow signed integer value. Let say the input value type range is [256, maxint]. Adding +128 will create 2 ranges due to overflow: [minint, minint+127] and [384, maxint]. But C2 type system keep only 1 type range and as result it use general [minint, maxint] for this case which we can't optimize.
>>
>> On other hand ConvI2L::Ideal(), used in load's address expression, does transformation ConvI2L[-128,127](AddI(i, low)) to AddL(ConvI2L[0,255](i), low_long). ConvI2L is special node because it records the range type of related array access. In this case type range of ConvI2L is [-128,127] and after ideal transformation it is [0,255]. ConvI2L::Value() produces TOP after the transformation because int types [0,255] and [256,maxint] do not meet. And as result the load also eliminated.
>>
>> We have a lot troubles with ConvI2L keeping a type which is valid only on some branch. The is RFE 6675699 to rework that (use CastII with control edge).
>>
>> The suggested fix for this bug is to check for overflow cases in range checks and collapse it if we can. In CmpU::Value() we look in types of inputs of AddI node and check both type ranges when there is overflow.
>>
>> I added code to put CmpU nodes on IGVN worklist if inputs of AddI were changed because AddI node may not be transformed (its type stays [minint, maxint] due to overflow).
>>
>> Enabled IfNode::fold_compares() optimization for all cases.
>>
>> Node notes were not constructed for new ideal nodes generated during post-parse inlining because C->default_node_notes() is set to NULL after Parse phase. Fix it by using root notes from the Call node we inline in do_late_inline().
>>
>> The problem in customer case happened when boxing methods (valueOf) are inlined after Parse. Post-parse inlining may case other problems. I decided to do that only under experimental aggressive unboxing option and work on it more in 9.
>>
>> I tested it with Scala build which showed the problem, ctw, jtreg, nashorn. I was not able to write a small test.
>>
>> I tried to measure performance impact of the fix with nashorn running octane benchmarks. It was less than variations between runs.
>>
>> Thanks,
>> Vladimir
>>
>>
>