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

[Vladimir Ivanov]

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
>>>>
>