RFR (XL): 8139170: JVMCI refresh
Vladimir Kozlov
vladimir.kozlov at oracle.com
Wed Oct 14 09:03:40 UTC 2015
>> 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
Changes looks good.
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