Preliminary RFR (L): 8001107: @Stable annotation for constant folding of lazily evaluated variables

[Vladimir Kozlov]

java/lang/invoke/MethodHandle.java has 0 diffs.

ciTypeArray.cpp: why we need these changes?

compile.cpp add {}:

+       if (flat->offset() == TypePtr::OffsetBot)
+         alias_type(idx)->set_element(flat->is_aryptr()->elem());


UseImplicitStableValues code is repeated 2 times. Could be moved into separate method.

memnode.cpp: Why in fold_stable_ary_elem() offset is taking from ary->offset() and nor from adr?

Can we have an assert in template interpreter that with FoldStableValues we only do 0/NULL -> non_0/not_NULL transition 
for @stable values?

In general seems fine.

Thanks,
Vladimir

On 8/15/13 2:05 AM, Vladimir Ivanov wrote:
> Chris, thank you for the review.
>
> I need one more review from a person wearing Reviewer hat :-) Thanks!
>
> Best regards,
> Vladimir Ivanov
>
> On 8/13/13 9:08 PM, Christian Thalinger wrote:
>> Looks good.  -- Chris
>>
>> On Jul 26, 2013, at 2:53 PM, Vladimir Ivanov <vladimir.x.ivanov at oracle.com> wrote:
>>
>>> Updated according to comments from Chris:
>>> http://cr.openjdk.java.net/~vlivanov/8001107/webrev.02
>>>
>>> Best regards,
>>> Vladimir Ivanov
>>>
>>> On 7/23/13 9:31 PM, Vladimir Ivanov wrote:
>>>> Rickard,
>>>>
>>>> Thank you for review.
>>>> See my answers inline.
>>>>
>>>> Updated version:
>>>> http://cr.openjdk.java.net/~vlivanov/8001107/webrev.01
>>>>
>>>> Best regards,
>>>> Vladimir Ivanov
>>>>
>>>> On 7/23/13 2:46 PM, Rickard Bäckman wrote:
>>>>> Vladimir,
>>>>>
>>>>> first, nice work.
>>>>>
>>>>> A couple of comments about the code:
>>>>>
>>>>> ciArray.hpp / ciArray.cpp
>>>>> Looks like most of the new methods could be declared const.
>>>> ciObject::klass() isn't const, so newly introduced methods can't be made
>>>> const as well.
>>>>
>>>>> ciConstant.hpp
>>>>> I would like to have braces add to the if / else if / else
>>>> Done.
>>>>
>>>>> classFileParser.hpp
>>>>> The is_stable() should be declared const.
>>>> Done.
>>>>
>>>>> memnode.cpp
>>>>> You are adding plenty of logic to an already very long method, could
>>>>> we extract the new parts to its own method? or methods.
>>>> Done. Introduced fold_stable_ary_elem helper function.
>>>>
>>>>> The elembt local variable is never really used.
>>>> Removed.
>>>>
>>>>> type.cpp
>>>>> 2 instances of assert(stable == true || stable == false, ""); - this
>>>>> seems to be always true?
>>>> Agree. Removed.
>>>>
>>>>> The constant 42 in the hash, since we are hashing pointers it might be
>>>>> more effective to use and odd number?
>>>> Done.
>>>>
>>>>> 3505: if (stable_dimension <= 0 || stable_dimension == 1 && stable ==
>>>>> this->stable())
>>>>> should have parantheses added to clarify the || && logic.
>>>> Done.
>>>>
>>>>> 3511   if (stable_dimension > 1 && elem_ptr != NULL &&
>>>>> elem_ptr->base() == Type::AryPtr) {
>>>>> could use the is_aryptr() instead of comparing base to Type::AryPtr
>>>> Done.
>>>>
>>>>> type.hpp
>>>>> the getter is named stable(), in all other places it is is_stable()
>>>>> maybe we could rename stable -> is_stable.
>>>> Done.
>>>>
>>>>> Cheers
>>>>> /R
>>>>>
>>>>> On Jul 20, 2013, at 12:19 AM, Vladimir Ivanov wrote:
>>>>>
>>>>>> http://cr.openjdk.java.net/~vlivanov/8001107/
>>>>>> 1455 lines changed: 1366 ins; 34 del; 55 mod;
>>>>>>
>>>>>> 8001107: @Stable annotation for constant folding of lazily evaluated
>>>>>> variables
>>>>>>
>>>>>> Excerpt from Javadoc:
>>>>>> [@Stable annotation is] Internal marker for some fields in the JSR
>>>>>> 292 implementation. A field may be annotated as stable if all of its
>>>>>> component variables changes value at most once. A field's value
>>>>>> counts as its component value. If the field is typed as an array,
>>>>>> then all the non-null components of the array, of depth up to the
>>>>>> rank of the field's array type, also count as component values. By
>>>>>> extension, any variable (either array or field) which has annotated
>>>>>> as stable is called a stable variable, and its non-null or non-zero
>>>>>> value is called a stable value.
>>>>>>
>>>>>> Testing: unit test, JPRT, VM testbase, nashorn, octane
>>>>>>
>>>>>> Contributed-by: jrose, vlivanov
>>>>>>
>>>>>> I marked review request as preliminary, because I think it may be too
>>>>>> late to get it into 8. It'll be targeted for 8update then.
>>>>>>
>>>>>> Thanks!
>>>>>>
>>>>>> Best regards,
>>>>>> Vladimir Ivanov
>>>>>
>>