RFR (XL): 8139170: JVMCI refresh

Tue Nov 3 06:37:39 UTC 2015

> On Oct 30, 2015, at 7:51 PM, Vladimir Kozlov <vladimir.kozlov at oracle.com> wrote:
> 
> On 10/31/15 6:43 AM, Christian Thalinger wrote:
>> 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/
> 
> Good.
> 
>> http://cr.openjdk.java.net/~twisti/8139170/hotspot/webrev/
> 
> Just push it already! It is hard to continue reviewing the same big changes over and over again. If something brakes we can fix it easy as small additional changes which easy to review.

Haha!  I know, sorry.  Igor is sending me new test fixes almost every day ;-)  I will integrate this tomorrow.

> 
> Thanks,
> Vladimir
> 
>> 
>> 
>>> 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.
>>>>>>>>> 
>>>>>>> 
>>>>>> 
>>>> 
>> 