RFR (XL): 8139170: JVMCI refresh

[Vladimir Kozlov]

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