RFR (XL): 8139170: JVMCI refresh
Christian Thalinger
christian.thalinger at oracle.com
Fri Oct 30 22:43:50 UTC 2015
Last update of the webrev with jdk9b88 and a few more test fixes. Note there are changes in the top-level directory now:
http://cr.openjdk.java.net/~twisti/8139170/webrev/
http://cr.openjdk.java.net/~twisti/8139170/hotspot/webrev/
> On Oct 23, 2015, at 8:09 PM, Vladimir Kozlov <vladimir.kozlov at oracle.com> wrote:
>
> 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.
>>>>>>>
>>>>>
>>>>
>>
More information about the hotspot-compiler-dev
mailing list