RFR (XL): 8139170: JVMCI refresh

[Vladimir Kozlov]

Good.

Thanks,
Vladimir

On 10/24/15 7:12 AM, Christian Thalinger wrote:
> I’ve updated the webrev with fixes from SQE for the tests which were failing due to changes the visibility of some internal classes and methods:
>
> http://cr.openjdk.java.net/~twisti/8139170/webrev/
>
>> On Oct 14, 2015, at 5:50 PM, Vladimir Kozlov <vladimir.kozlov at oracle.com> wrote:
>>
>> Okay.
>>
>> Thanks,
>> Vladimir
>>
>> On 10/15/15 8:37 AM, Christian Thalinger wrote:
>>> I have pulled two SPARC changes:
>>>
>>> http://lafo.ssw.uni-linz.ac.at/hg/graal-jvmci-9/rev/c158981b3c59
>>> http://lafo.ssw.uni-linz.ac.at/hg/graal-jvmci-9/rev/110a130aa88b
>>>
>>> and updated the webrev to include (actually exclude) the individual bug fixes pushed to hs-comp.
>>>
>>>> On Oct 14, 2015, at 8:55 AM, Christian Thalinger <christian.thalinger at oracle.com> wrote:
>>>>
>>>>
>>>>> On Oct 13, 2015, at 11:03 PM, Vladimir Kozlov <vladimir.kozlov at oracle.com> wrote:
>>>>>
>>>>>>> It would be good if we would run at least one JVMCI test in JPRT.
>>>>>
>>>>> If they are stable you can add them (compiler/jvmci for all) to hotspot_compiler_3 in test/TEST.groups
>>>>
>>>> I’ll let Igor I. do that.
>>>>
>>>>>
>>>>> Changes looks good.
>>>>
>>>> Thanks.
>>>>
>>>>>
>>>>> Thanks,
>>>>> Vladimir
>>>>>
>>>>> On 10/14/15 2:31 AM, Christian Thalinger wrote:
>>>>>> webrev updated.
>>>>>>
>>>>>>> On Oct 13, 2015, at 8:01 AM, Christian Thalinger
>>>>>>> <christian.thalinger at oracle.com
>>>>>>> <mailto:christian.thalinger at oracle.com>> wrote:
>>>>>>>
>>>>>>>>
>>>>>>>> On Oct 12, 2015, at 10:59 PM, Doug Simon <doug.simon at oracle.com
>>>>>>>> <mailto:doug.simon at oracle.com>> wrote:
>>>>>>>>
>>>>>>>>>
>>>>>>>>> On 13 Oct 2015, at 06:19, Vladimir Kozlov
>>>>>>>>> <vladimir.kozlov at oracle.com <mailto:vladimir.kozlov at oracle.com>> wrote:
>>>>>>>>>
>>>>>>>>> Since we will get more changes from labs later we may enumerate
>>>>>>>>> them: JVMCI refresh 1.
>>>>>>>>
>>>>>>>> Or, pessimistically, JVMCI refresh 01 ;-)
>>>>>>>>
>>>>>>>>> Can you explain new sed command in Gensrc-jdk.vm.ci.gmk.
>>>>>>>>
>>>>>>>> http://hg.openjdk.java.net/graal/graal-jvmci-8/rev/1852abfbaca3
>>>>>>>>
>>>>>>>>> Are you sure it works on windows?
>>>>>>>>
>>>>>>>> I can’t say for sure but given that I see similar patterns in other
>>>>>>>> *.gmk files, I think it should be fine.
>>>>>>>
>>>>>>> It would be good if we would run at least one JVMCI test in JPRT.
>>>>>>>
>>>>>>>>
>>>>>>>>> Instead of is_trivial(method) may be we should have general function
>>>>>>>>> called from AdvancedThresholdPolicy::common() which return
>>>>>>>>> compilation level for particular method (for example, we can also
>>>>>>>>> limit it using compilecommand). Could be done as separate change
>>>>>>>>> after this.
>>>>>>>>>
>>>>>>>>> I looked on jdk.vm.ci changes and nothing looks terrible wrong. I am
>>>>>>>>> not familiar with it so I can't say that each line is correct. One
>>>>>>>>> thing I am wondering is why explicit imports are used instead of .*
>>>>>>>>> (compilation speed?):
>>>>>>>>
>>>>>>>> http://mail.openjdk.java.net/pipermail/graal-dev/2015-September/003546.html
>>>>>>>>
>>>>>>>>> -import java.lang.annotation.*;
>>>>>>>>> +import java.lang.annotation.ElementType;
>>>>>>>>> +import java.lang.annotation.Retention;
>>>>>>>>> +import java.lang.annotation.RetentionPolicy;
>>>>>>>>> +import java.lang.annotation.Target;
>>>>>>>>>
>>>>>>>>> An other thing is using AMD64/amd64 in files and directory names.
>>>>>>>>> May be we should rename it to x64.
>>>>>>>>>
>>>>>>>>> jvmciCodeInstaller_x86.cpp and jvmciCodeInstaller.cpp has next pattern:
>>>>>>>>> +      if (HotSpotMetaspaceConstantImpl::compressed(constant)) {
>>>>>>>>> +#ifdef _LP64
>>>>>>>>> +        ...
>>>>>>>>> +#else
>>>>>>>>> +        fatal("unexpected compressed Klass* in 32-bit mode");
>>>>>>>>> +#endif
>>>>>>>>> +  } else {
>>>>>>>>>
>>>>>>>>> It would be nice to hide fatal() in some wrapper function.
>>>>>>>>>
>>>>>>>>> jvmciEnv.cpp use ASSERT instead of DEBUG. We renamed ifdef DEBUG
>>>>>>>>> long ago.
>>>>>>>>>
>>>>>>>>> globals.hpp changes conflict with 8139377.
>>>>>>>>
>>>>>>>> Then we should adopt 8139377 instead.
>>>>>>>
>>>>>>> When I created the review 8139377 was not integrated yet.  Let me pull
>>>>>>> again.
>>>>>>>
>>>>>>>>
>>>>>>>>> Tests changes (mostly renaming) looks fine.
>>>>>>>>>
>>>>>>>>> Fix copyright years (2015, 2015) in new files.
>>>>>>>>>
>>>>>>>>> Thanks,
>>>>>>>>> Vladimir
>>>>>>>>>
>>>>>>>>> On 10/13/15 10:19 AM, Christian Thalinger wrote:
>>>>>>>>>> https://bugs.openjdk.java.net/browse/JDK-8139170
>>>>>>>>>> http://cr.openjdk.java.net/~twisti/8139170/webrev/
>>>>>>>>>>
>>>>>>>>>> During the review period for JEP 243 there were some changes and
>>>>>>>>>> enhancements to the JVMCI code done by Oracle Labs. In order to not
>>>>>>>>>> disturb the already long and complicated review of JEP 243 we
>>>>>>>>>> decided to do a refresh after the initial integration.
>>>>>>>>>>
>>>>>>>>>> A lot of the Java changes is switching to using explicit imports.
>>>>>>
>>>>
>>>
>