RFR (XL): 8139170: JVMCI refresh

[Christian Thalinger]

webrev updated.

> On Oct 13, 2015, at 8:01 AM, Christian Thalinger <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 <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 <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 <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.

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/attachments/20151013/d49fb400/attachment-0001.html>