RFR (XL): 8139170: JVMCI refresh

Christian Thalinger christian.thalinger at oracle.com
Wed Oct 14 18:55:02 UTC 2015


> 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