RFR (XL): 8139170: JVMCI refresh

Christian Thalinger christian.thalinger at oracle.com
Fri Oct 23 23:12:11 UTC 2015


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



More information about the hotspot-compiler-dev mailing list