[9] RFR(XL) 8166417: Integrate Graal-core into JDK for AOT compiler

Sat Dec 10 07:42:35 UTC 2016

This looks ok to me.

/Erik

On 2016-12-09 10:13, Vladimir Kozlov wrote:
> Thank you, Magnus, for review.
>
> I updated webrevs with merge from latest jdk9/hs which now has Jigsaw 
> update.
>
> http://cr.openjdk.java.net/~kvn/8166417.v2/top.webrev/
> http://cr.openjdk.java.net/~kvn/8166417.v2/jdk.webrev/
> http://cr.openjdk.java.net/~kvn/8166417.v2/hotspot.webrev/
>
> Removed hotspot/make/Tools.gmk
>
> Removed all overview.html files in graal sources.
>
> Fixed hotspot.m4 and changed condition in hotspot.m4 to build graal 
> only on linux-x64. The condition will be changed to check ENABLE_AOT 
> when merged with AOT changes later.
>
> Moved module-info.java.extra from share/classes to unix/classes per 
> Mandy suggestion since we will build graal only for AOT.
>
> I have to modify make/Javadoc.gmk to avoid next JPRT failure:
>
> javadoc: error - No source files for package 
> org.graalvm.compiler.api.collections.src.org.graalvm.compiler.api.collections
> make[3]: *** 
> [/scratch/opt/jprt/T/P1/232453.vkozlov/s/build/linux-x64/images/docs/api/index.html] 
> Error 1
>
> We did not generate docs for Graal before because its package was 
> com.oracle.graal. I think it is fine to not generate it now too.
> Please, help if you think make/Javadoc.gmk should be fixed other way.
>
> By Mandy request changed code in 
> hotspot/make/gensrc/Gensrc-jdk.vm.compiler.gmk to produce 
> module-info.java.extra with one `provides` clause of the same SPI like 
> next:
>
> provides org.graalvm.compiler.hotspot.CompilerConfigurationFactory with
>     org.graalvm.compiler.hotspot.CoreCompilerConfigurationFactory,
> org.graalvm.compiler.hotspot.EconomyCompilerConfigurationFactory,
>     ;
>
>
> About moving preprocessors .java files into make/src and .test files 
> into hotspot/test.
>
> First, they are unit tests and not jtreg. Second, we did experiment 
> during AOT development with tests move but found that it is merge 
> (from graal-core) nightmare and tests execution problems.
>
> For now we want sources in JDK exactly match sources in 
> https://github.com/graalvm/graal-core
>
> Oracle Labs continue Graal development very actively and we want to be 
> in sync with them without big merge overhead.
>
> Thanks,
> Vladimir
>
> On 12/8/16 5:08 AM, Doug Simon wrote:
>>
>>> On 8 Dec 2016, at 13:50, Magnus Ihse Bursie 
>>> <magnus.ihse.bursie at oracle.com> wrote:
>>>
>>> On 2016-12-07 23:10, Vladimir Kozlov wrote:
>>>> https://bugs.openjdk.java.net/browse/JDK-8166417
>>>>
>>>> It is part of JEP 295: Ahead-of-Time Compilation
>>>> https://bugs.openjdk.java.net/browse/JDK-8166089
>>>>
>>>> http://cr.openjdk.java.net/~kvn/8166417/top.webrev/
>>>> http://cr.openjdk.java.net/~kvn/8166417/jdk.webrev/
>>>> http://cr.openjdk.java.net/~kvn/8166417/hotspot.webrev/
>>>
>>> * In CompileJavaModules.gmk, there is an *extreme* list of excludes. 
>>> I note that most of them end in ".test". The proper solution to this 
>>> is to move the test code to the test/ directory instead of the src/ 
>>> directory. Also, four of them are the javac processor build tools. 
>>> These should move to make/src, as other build tools.
>>
>> Keeping like it is simplifies pulling changes from 
>> https://github.com/graalvm/graal-core. It's possible to write a more 
>> sophisticated tool for pulling changes in from the github and places 
>> the sources into more openjdk compatible locations. It's up to the 
>> HotSpot compiler team whether investing in such a tool makes sense.
>>
>>> * In hotspot.m4: AC_MSG_CHECKING must always be matched by a 
>>> AC_MSG_RESULT, even before AC_MSG_ERROR, otherwise the output will 
>>> be garbled.
>>>
>>> * In hotspot.m4: The test to determine if we should set 
>>> INCLUDE_GRAAL is incorrect. It only tests for the default value, not 
>>> for the explicitely set (i.e., HOTSPOT_CHECK_JVM_FEATURE(graal)).
>>>
>>> * The file hotspot/make/Tools.gmk is broken ("ifeq ($(ENABLE_AOT), 
>>> true)" will never evaluate to true). But on the other hand, it is 
>>> not needed, and should just be removed. :-)
>>>
>>> * The file hotspot/src/jdk.vm.compiler/share/classes/overview.html 
>>> seems a bit dangling. Is this supposed to be included in some 
>>> Javadoc generation? The html file refers to a projects.jpg and 
>>> projects.html that does not exist. Maybe it should just be removed?
>>
>> I would recommend omitting all graal-core overview.html files in 
>> OpenJDK.
>>
>>> * Finally, just for the record, I too think the source should be 
>>> structured according to the OpenJDK praxis. (But I won't fight about 
>>> this.)
>>
>> -Doug
>>
>>>>
>>>> This is formal review request for integration Graal-core sources 
>>>> into OpenJDK. AOT compiler uses Graal-core as backend compiler. We 
>>>> need to integrated Graal-core sources into JDK and add build 
>>>> changes to build Graal module.
>>>>
>>>> Note, changes are based on latest jdk9/hs sources which do not have 
>>>> latest jigsaw update yet. With jigsaw update small changes will be 
>>>> done to module-info.java.extra in java.base:
>>>>
>>>>  exports jdk.internal.misc to jdk.vm.compiler;
>>>> + opens jdk.internal.misc to jdk.vm.compiler;
>>>>
>>>> - exports com.sun.crypto.provider to jdk.vm.compiler;
>>>> + opens com.sun.crypto.provider to jdk.vm.compiler;
>>>>
>>>> And changes in top make/GensrcModuleInfo.gmk will not be needed.
>>>>
>>>>
>>>>
>>>> Graal is a dynamic compiler written in Java that integrates with 
>>>> the HotSpot JVM. It has a focus on high performance and 
>>>> extensibility. In addition, it provides optimized performance for 
>>>> Truffle based languages running on the JVM.
>>>>
>>>> https://github.com/graalvm/graal-core
>>>>
>>>> Oracle Labs is developing and maintaining it.
>>>>
>>>> Here are people who contributed into Graal development (sorry if 
>>>> someone is missing or misspelled, please speak):
>>>>
>>>> ~70k LOC: Douglas Simon
>>>> ~60k LOC: Lukas Stadler
>>>> ~30k LOC: Thomas Wuerthinger
>>>> ~30k LOC: Tom Rodriguez
>>>> ~30k LOC: Roland Schatz
>>>> ~30k LOC: Josef Eisl
>>>> ~30k LOC: Christian Wimmer
>>>> ~16k LOC: Chris Thalinger
>>>> ~13k LOC: Gilles Duboscq
>>>> ~11k LOC: David Leopoldseder
>>>> ~ 8k LOC: Stefan Anzinger
>>>> ~ 8k LOC: Christian Humer
>>>>
>>>> Other contributors >100 LOC in approximate order of contribution size:
>>>> Michael Berg, Bernhard Urban, Miguel Garcia, Yudi Zheng, Christos 
>>>> Kotselidis, Andreas Woess, Stefan Rumzucker, Aleksandar Prokopec, 
>>>> Christian Haeubl, Morris Meyer, Matthias Grimmer, Erik Eckstein, 
>>>> Josef Haider, Manuel Rigger, Michael Haupt, Niclas Adlertz, 
>>>> Jaroslav Tulach, Chris Seaton, Peter B. Kessler, Christian Wirth, 
>>>> Benoit Daloze.
>>>>
>>>>
>>>> Thanks,
>>>> Vladimir
>>>
>>