Re: RFR: JDK-8142907 Integration of minor fixes from the build-infra project
On 14/12/2015 6:30 PM, Magnus Ihse Bursie wrote: On 2015-12-14 09:12, David Holmes wrote: Hi Magnus, On 12/12/2015 8:24 AM, Magnus Ihse Bursie wrote: On 2015-12-11 22:43, Magnus Ihse Bursie wrote: Hi David, Resurrecting the second part of the build-infra integrations. :) Fortunately, no major code changes in the meantime has affected this patch. (I did need to update the new hotspot/make/lib/Lib-jdk.hotspot.agent.gmk though.) It seems I forgot to include the link to the new webrev: http://cr.openjdk.java.net/~ihse/JDK-8142907-build-infra-integration-closed/webrev.05 hotspot.m4: 54 #server: normal interpreter and a tiered C1/C2 compiler 55 #client: normal interpreter and C1 (no C2 compiler) (only 32-bit platforms) Neither of these statements are necessarily 100% true - not all platforms have tiered enable; some 64-bit platforms do (or will) support client. This code was directly moved from jdk-options.m4. But I can update the text to better fit reality. I suggest updating the comment about server to: # server: normal interpreter, and a C2 or tiered C1/C2 compiler depending on platform Ok? Ok. However, a few lines below we explicitely block enabling of client on 64-bit platforms, so I believe this statement is correct, at least for the code as it looks now. It might be the case that the configure check is too strict, and if so we should modify it, but as long as the check remains I believe the comment should remain. Ok? OK. As long as whomever removes the check knows to remove the comment. Both the arm32 port and the mobile project will be relaxing this afaik. Thanks, David Otherwise I didn't see anything else that I obviously needed to comment on. The proof of this is in the building. So as long as this gets through JPRT with -testset hotspot we should be okay. :) Great. Thanks for the review! /Magnus Thanks, David /Magnus On 2015-11-16 05:33, David Holmes wrote: Hi Magnus, I had a flick through most of the files. Overall seems okay but I have a few queries below. Replies inline. On 13/11/2015 12:43 PM, Magnus Ihse Bursie wrote: The build-infra project has collected a number of minor fixes and changes during the new hotspot build development. Its a mix of code cleanup and new capabilities. Not all of these new features are immediately beneficial to the JDK, but they will be needed for the upcoming new Hotspot build, and it will not hurt to have them in mainline. (In fact, it will tremendously help merging between mainline and build-infra.) The fix addresses these issues: In general: * Break out hotspot configuration into hotspot.m4 * Long link lines uses @-files * Consistently use -Wl instead of -Xlinker * Improve clang on linux compilation * Set shared library name explicitely on solaris * Set correct shared library flag on Windows (-dll) * Consistency fixes for build toolchain * Bring compare script up to date * General code/whitespace cleanup * Additional functionality in MakeBase In NativeCompilation.gmk: * More efficient vardeps for per-file CFLAGS * Fewer shell executions (means better performance on Windows) * EXCLUDE_PATTERN and EXTRA_OBJECT_FILES * Debug symbols on macosx (disabled for existing code to keep current behavior) Enabling debug info on macosx on existing jdk should be treated in a follow-up bug. Bug: https://bugs.openjdk.java.net/browse/JDK-8142907 WebRev: http://cr.openjdk.java.net/~ihse/JDK-8142907-build-infra-integration-closed/webrev.01 (It turned out that WebRev could not at the same time include files from multiple repos and track the history of a "hg cp":ied file. So I created an alternative revision here: http://cr.openjdk.java.net/~ihse/JDK-8142907-build-infra-integration-closed/webrev.02/ It does not include the jdk files, but hotspot.m4 might be easier to understand) flags.m4: 60 AC_SUBST(LEGACY_EXTRA_CFLAGS) 61 AC_SUBST(LEGACY_EXTRA_CXXFLAGS) 62 AC_SUBST(LEGACY_EXTRA_LDFLAGS) 63 64 AC_SUBST(EXTRA_CFLAGS) 65 AC_SUBST(EXTRA_CXXFLAGS) 66 AC_SUBST(EXTRA_LDFLAGS) IIRC we added the legacy flags purely to pass the cross-compilation args through to hotspot. Not sure why we need both legacy and non-legancy variants now ?? This is part of an ongoing effort to split CFLAGS_JDK into more reasonable parts. The current model is that we throw all possible flags together in CFLAGS_JDK (and then further on to CFLAGS_JDKLIB et al). While it was questionable even before, it did work when we only built the JDK native libraries. Now that we need to build libjvm.so as well, which has (for historical reason) a set of flags that are both partially the same and partially different, this is not so good. We would like to see a change to a situation where the different "parts" that we build CFLAGS_JDK from would be handled separately, and then combined as needed for JDK libraries, and for Hotspot. So, currently we use EXTRA_CFLAGS just to add them to CFLAGS_JDK, and then drop them. In the new
Re: RFR: JDK-8142907 Integration of minor fixes from the build-infra project
On 12/14/15 07:34, David Holmes wrote: On 14/12/2015 6:30 PM, Magnus Ihse Bursie wrote: On 2015-12-14 09:12, David Holmes wrote: Hi Magnus, On 12/12/2015 8:24 AM, Magnus Ihse Bursie wrote: On 2015-12-11 22:43, Magnus Ihse Bursie wrote: Hi David, Resurrecting the second part of the build-infra integrations. :) Fortunately, no major code changes in the meantime has affected this patch. (I did need to update the new hotspot/make/lib/Lib-jdk.hotspot.agent.gmk though.) It seems I forgot to include the link to the new webrev: http://cr.openjdk.java.net/~ihse/JDK-8142907-build-infra-integration-closed/webrev.05 hotspot.m4: 54 #server: normal interpreter and a tiered C1/C2 compiler 55 #client: normal interpreter and C1 (no C2 compiler) (only 32-bit platforms) Neither of these statements are necessarily 100% true - not all platforms have tiered enable; some 64-bit platforms do (or will) support client. This code was directly moved from jdk-options.m4. But I can update the text to better fit reality. I suggest updating the comment about server to: # server: normal interpreter, and a C2 or tiered C1/C2 compiler depending on platform Ok? Ok. However, a few lines below we explicitely block enabling of client on 64-bit platforms, so I believe this statement is correct, at least for the code as it looks now. It might be the case that the configure check is too strict, and if so we should modify it, but as long as the check remains I believe the comment should remain. Ok? OK. As long as whomever removes the check knows to remove the comment. Both the arm32 port and the mobile project will be relaxing this afaik. For the mobile port we have relaxed the constraint and allow a 64 bit client and minimal vm to be built. I'll make sure the comment is updated in our repos. I don't think there was a fundamental reason those variations were not support, just a question of which platforms were being built and tested on a regular basis. Thanks, David Otherwise I didn't see anything else that I obviously needed to comment on. The proof of this is in the building. So as long as this gets through JPRT with -testset hotspot we should be okay. :) Great. Thanks for the review! /Magnus Thanks, David /Magnus On 2015-11-16 05:33, David Holmes wrote: Hi Magnus, I had a flick through most of the files. Overall seems okay but I have a few queries below. Replies inline. On 13/11/2015 12:43 PM, Magnus Ihse Bursie wrote: The build-infra project has collected a number of minor fixes and changes during the new hotspot build development. Its a mix of code cleanup and new capabilities. Not all of these new features are immediately beneficial to the JDK, but they will be needed for the upcoming new Hotspot build, and it will not hurt to have them in mainline. (In fact, it will tremendously help merging between mainline and build-infra.) The fix addresses these issues: In general: * Break out hotspot configuration into hotspot.m4 * Long link lines uses @-files * Consistently use -Wl instead of -Xlinker * Improve clang on linux compilation * Set shared library name explicitely on solaris * Set correct shared library flag on Windows (-dll) * Consistency fixes for build toolchain * Bring compare script up to date * General code/whitespace cleanup * Additional functionality in MakeBase In NativeCompilation.gmk: * More efficient vardeps for per-file CFLAGS * Fewer shell executions (means better performance on Windows) * EXCLUDE_PATTERN and EXTRA_OBJECT_FILES * Debug symbols on macosx (disabled for existing code to keep current behavior) Enabling debug info on macosx on existing jdk should be treated in a follow-up bug. Bug: https://bugs.openjdk.java.net/browse/JDK-8142907 WebRev: http://cr.openjdk.java.net/~ihse/JDK-8142907-build-infra-integration-closed/webrev.01 (It turned out that WebRev could not at the same time include files from multiple repos and track the history of a "hg cp":ied file. So I created an alternative revision here: http://cr.openjdk.java.net/~ihse/JDK-8142907-build-infra-integration-closed/webrev.02/ It does not include the jdk files, but hotspot.m4 might be easier to understand) flags.m4: 60 AC_SUBST(LEGACY_EXTRA_CFLAGS) 61 AC_SUBST(LEGACY_EXTRA_CXXFLAGS) 62 AC_SUBST(LEGACY_EXTRA_LDFLAGS) 63 64 AC_SUBST(EXTRA_CFLAGS) 65 AC_SUBST(EXTRA_CXXFLAGS) 66 AC_SUBST(EXTRA_LDFLAGS) IIRC we added the legacy flags purely to pass the cross-compilation args through to hotspot. Not sure why we need both legacy and non-legancy variants now ?? This is part of an ongoing effort to split CFLAGS_JDK into more reasonable parts. The current model is that we throw all possible flags together in CFLAGS_JDK (and then further on to CFLAGS_JDKLIB et al). While it was questionable even before, it did work when we only built the JDK native libraries. Now that we need to build libjvm.so as well, which has (for historical reason) a set
Re: RFR: JDK-8142907 Integration of minor fixes from the build-infra project
Hi Magnus, On 12/12/2015 8:24 AM, Magnus Ihse Bursie wrote: On 2015-12-11 22:43, Magnus Ihse Bursie wrote: Hi David, Resurrecting the second part of the build-infra integrations. :) Fortunately, no major code changes in the meantime has affected this patch. (I did need to update the new hotspot/make/lib/Lib-jdk.hotspot.agent.gmk though.) It seems I forgot to include the link to the new webrev: http://cr.openjdk.java.net/~ihse/JDK-8142907-build-infra-integration-closed/webrev.05 hotspot.m4: 54 #server: normal interpreter and a tiered C1/C2 compiler 55 #client: normal interpreter and C1 (no C2 compiler) (only 32-bit platforms) Neither of these statements are necessarily 100% true - not all platforms have tiered enable; some 64-bit platforms do (or will) support client. Otherwise I didn't see anything else that I obviously needed to comment on. The proof of this is in the building. So as long as this gets through JPRT with -testset hotspot we should be okay. :) Thanks, David /Magnus On 2015-11-16 05:33, David Holmes wrote: Hi Magnus, I had a flick through most of the files. Overall seems okay but I have a few queries below. Replies inline. On 13/11/2015 12:43 PM, Magnus Ihse Bursie wrote: The build-infra project has collected a number of minor fixes and changes during the new hotspot build development. Its a mix of code cleanup and new capabilities. Not all of these new features are immediately beneficial to the JDK, but they will be needed for the upcoming new Hotspot build, and it will not hurt to have them in mainline. (In fact, it will tremendously help merging between mainline and build-infra.) The fix addresses these issues: In general: * Break out hotspot configuration into hotspot.m4 * Long link lines uses @-files * Consistently use -Wl instead of -Xlinker * Improve clang on linux compilation * Set shared library name explicitely on solaris * Set correct shared library flag on Windows (-dll) * Consistency fixes for build toolchain * Bring compare script up to date * General code/whitespace cleanup * Additional functionality in MakeBase In NativeCompilation.gmk: * More efficient vardeps for per-file CFLAGS * Fewer shell executions (means better performance on Windows) * EXCLUDE_PATTERN and EXTRA_OBJECT_FILES * Debug symbols on macosx (disabled for existing code to keep current behavior) Enabling debug info on macosx on existing jdk should be treated in a follow-up bug. Bug: https://bugs.openjdk.java.net/browse/JDK-8142907 WebRev: http://cr.openjdk.java.net/~ihse/JDK-8142907-build-infra-integration-closed/webrev.01 (It turned out that WebRev could not at the same time include files from multiple repos and track the history of a "hg cp":ied file. So I created an alternative revision here: http://cr.openjdk.java.net/~ihse/JDK-8142907-build-infra-integration-closed/webrev.02/ It does not include the jdk files, but hotspot.m4 might be easier to understand) flags.m4: 60 AC_SUBST(LEGACY_EXTRA_CFLAGS) 61 AC_SUBST(LEGACY_EXTRA_CXXFLAGS) 62 AC_SUBST(LEGACY_EXTRA_LDFLAGS) 63 64 AC_SUBST(EXTRA_CFLAGS) 65 AC_SUBST(EXTRA_CXXFLAGS) 66 AC_SUBST(EXTRA_LDFLAGS) IIRC we added the legacy flags purely to pass the cross-compilation args through to hotspot. Not sure why we need both legacy and non-legancy variants now ?? This is part of an ongoing effort to split CFLAGS_JDK into more reasonable parts. The current model is that we throw all possible flags together in CFLAGS_JDK (and then further on to CFLAGS_JDKLIB et al). While it was questionable even before, it did work when we only built the JDK native libraries. Now that we need to build libjvm.so as well, which has (for historical reason) a set of flags that are both partially the same and partially different, this is not so good. We would like to see a change to a situation where the different "parts" that we build CFLAGS_JDK from would be handled separately, and then combined as needed for JDK libraries, and for Hotspot. So, currently we use EXTRA_CFLAGS just to add them to CFLAGS_JDK, and then drop them. In the new Hotspot build, we use the EXTRA_CFLAGS to add them to the Hotspot flags. Of course, we could have used LEGACY_EXTRA_CFLAGS, but we'd like to avoid using "LEGACY" for new stuff. With that being said, I now realize that maybe this change will not be needed anyway, at least not right now. Since doing a proper flag cleanup is a major task, we decided to copy the CFLAGS_JDK behavior for Hotspot flags as well in the current implementation of the new hotspot build, and saving the cleanup for another day (so it will not block the new build system). So I'll revert this part of the change for now. On Windows -LD is a superset of -dll, so it isn't obvious the change is correct. I hope Erik's reply to that was satisfactory. --- jdk/make/lib/LibCommon.gmk + # Disable it here for the jdk binaries until we decide to enable them. s/binaries/libraries/ ? Sure, I'll fix.
Re: RFR: JDK-8142907 Integration of minor fixes from the build-infra project
On 2015-12-14 09:12, David Holmes wrote: Hi Magnus, On 12/12/2015 8:24 AM, Magnus Ihse Bursie wrote: On 2015-12-11 22:43, Magnus Ihse Bursie wrote: Hi David, Resurrecting the second part of the build-infra integrations. :) Fortunately, no major code changes in the meantime has affected this patch. (I did need to update the new hotspot/make/lib/Lib-jdk.hotspot.agent.gmk though.) It seems I forgot to include the link to the new webrev: http://cr.openjdk.java.net/~ihse/JDK-8142907-build-infra-integration-closed/webrev.05 hotspot.m4: 54 #server: normal interpreter and a tiered C1/C2 compiler 55 #client: normal interpreter and C1 (no C2 compiler) (only 32-bit platforms) Neither of these statements are necessarily 100% true - not all platforms have tiered enable; some 64-bit platforms do (or will) support client. This code was directly moved from jdk-options.m4. But I can update the text to better fit reality. I suggest updating the comment about server to: # server: normal interpreter, and a C2 or tiered C1/C2 compiler depending on platform Ok? However, a few lines below we explicitely block enabling of client on 64-bit platforms, so I believe this statement is correct, at least for the code as it looks now. It might be the case that the configure check is too strict, and if so we should modify it, but as long as the check remains I believe the comment should remain. Ok? Otherwise I didn't see anything else that I obviously needed to comment on. The proof of this is in the building. So as long as this gets through JPRT with -testset hotspot we should be okay. :) Great. Thanks for the review! /Magnus Thanks, David /Magnus On 2015-11-16 05:33, David Holmes wrote: Hi Magnus, I had a flick through most of the files. Overall seems okay but I have a few queries below. Replies inline. On 13/11/2015 12:43 PM, Magnus Ihse Bursie wrote: The build-infra project has collected a number of minor fixes and changes during the new hotspot build development. Its a mix of code cleanup and new capabilities. Not all of these new features are immediately beneficial to the JDK, but they will be needed for the upcoming new Hotspot build, and it will not hurt to have them in mainline. (In fact, it will tremendously help merging between mainline and build-infra.) The fix addresses these issues: In general: * Break out hotspot configuration into hotspot.m4 * Long link lines uses @-files * Consistently use -Wl instead of -Xlinker * Improve clang on linux compilation * Set shared library name explicitely on solaris * Set correct shared library flag on Windows (-dll) * Consistency fixes for build toolchain * Bring compare script up to date * General code/whitespace cleanup * Additional functionality in MakeBase In NativeCompilation.gmk: * More efficient vardeps for per-file CFLAGS * Fewer shell executions (means better performance on Windows) * EXCLUDE_PATTERN and EXTRA_OBJECT_FILES * Debug symbols on macosx (disabled for existing code to keep current behavior) Enabling debug info on macosx on existing jdk should be treated in a follow-up bug. Bug: https://bugs.openjdk.java.net/browse/JDK-8142907 WebRev: http://cr.openjdk.java.net/~ihse/JDK-8142907-build-infra-integration-closed/webrev.01 (It turned out that WebRev could not at the same time include files from multiple repos and track the history of a "hg cp":ied file. So I created an alternative revision here: http://cr.openjdk.java.net/~ihse/JDK-8142907-build-infra-integration-closed/webrev.02/ It does not include the jdk files, but hotspot.m4 might be easier to understand) flags.m4: 60 AC_SUBST(LEGACY_EXTRA_CFLAGS) 61 AC_SUBST(LEGACY_EXTRA_CXXFLAGS) 62 AC_SUBST(LEGACY_EXTRA_LDFLAGS) 63 64 AC_SUBST(EXTRA_CFLAGS) 65 AC_SUBST(EXTRA_CXXFLAGS) 66 AC_SUBST(EXTRA_LDFLAGS) IIRC we added the legacy flags purely to pass the cross-compilation args through to hotspot. Not sure why we need both legacy and non-legancy variants now ?? This is part of an ongoing effort to split CFLAGS_JDK into more reasonable parts. The current model is that we throw all possible flags together in CFLAGS_JDK (and then further on to CFLAGS_JDKLIB et al). While it was questionable even before, it did work when we only built the JDK native libraries. Now that we need to build libjvm.so as well, which has (for historical reason) a set of flags that are both partially the same and partially different, this is not so good. We would like to see a change to a situation where the different "parts" that we build CFLAGS_JDK from would be handled separately, and then combined as needed for JDK libraries, and for Hotspot. So, currently we use EXTRA_CFLAGS just to add them to CFLAGS_JDK, and then drop them. In the new Hotspot build, we use the EXTRA_CFLAGS to add them to the Hotspot flags. Of course, we could have used LEGACY_EXTRA_CFLAGS, but we'd like to avoid using "LEGACY" for new stuff. With that being said, I
Re: RFR: JDK-8142907 Integration of minor fixes from the build-infra project
Looks good to me. /Erik On 2015-12-11 23:24, Magnus Ihse Bursie wrote: On 2015-12-11 22:43, Magnus Ihse Bursie wrote: Hi David, Resurrecting the second part of the build-infra integrations. :) Fortunately, no major code changes in the meantime has affected this patch. (I did need to update the new hotspot/make/lib/Lib-jdk.hotspot.agent.gmk though.) It seems I forgot to include the link to the new webrev: http://cr.openjdk.java.net/~ihse/JDK-8142907-build-infra-integration-closed/webrev.05 /Magnus On 2015-11-16 05:33, David Holmes wrote: Hi Magnus, I had a flick through most of the files. Overall seems okay but I have a few queries below. Replies inline. On 13/11/2015 12:43 PM, Magnus Ihse Bursie wrote: The build-infra project has collected a number of minor fixes and changes during the new hotspot build development. Its a mix of code cleanup and new capabilities. Not all of these new features are immediately beneficial to the JDK, but they will be needed for the upcoming new Hotspot build, and it will not hurt to have them in mainline. (In fact, it will tremendously help merging between mainline and build-infra.) The fix addresses these issues: In general: * Break out hotspot configuration into hotspot.m4 * Long link lines uses @-files * Consistently use -Wl instead of -Xlinker * Improve clang on linux compilation * Set shared library name explicitely on solaris * Set correct shared library flag on Windows (-dll) * Consistency fixes for build toolchain * Bring compare script up to date * General code/whitespace cleanup * Additional functionality in MakeBase In NativeCompilation.gmk: * More efficient vardeps for per-file CFLAGS * Fewer shell executions (means better performance on Windows) * EXCLUDE_PATTERN and EXTRA_OBJECT_FILES * Debug symbols on macosx (disabled for existing code to keep current behavior) Enabling debug info on macosx on existing jdk should be treated in a follow-up bug. Bug: https://bugs.openjdk.java.net/browse/JDK-8142907 WebRev: http://cr.openjdk.java.net/~ihse/JDK-8142907-build-infra-integration-closed/webrev.01 (It turned out that WebRev could not at the same time include files from multiple repos and track the history of a "hg cp":ied file. So I created an alternative revision here: http://cr.openjdk.java.net/~ihse/JDK-8142907-build-infra-integration-closed/webrev.02/ It does not include the jdk files, but hotspot.m4 might be easier to understand) flags.m4: 60 AC_SUBST(LEGACY_EXTRA_CFLAGS) 61 AC_SUBST(LEGACY_EXTRA_CXXFLAGS) 62 AC_SUBST(LEGACY_EXTRA_LDFLAGS) 63 64 AC_SUBST(EXTRA_CFLAGS) 65 AC_SUBST(EXTRA_CXXFLAGS) 66 AC_SUBST(EXTRA_LDFLAGS) IIRC we added the legacy flags purely to pass the cross-compilation args through to hotspot. Not sure why we need both legacy and non-legancy variants now ?? This is part of an ongoing effort to split CFLAGS_JDK into more reasonable parts. The current model is that we throw all possible flags together in CFLAGS_JDK (and then further on to CFLAGS_JDKLIB et al). While it was questionable even before, it did work when we only built the JDK native libraries. Now that we need to build libjvm.so as well, which has (for historical reason) a set of flags that are both partially the same and partially different, this is not so good. We would like to see a change to a situation where the different "parts" that we build CFLAGS_JDK from would be handled separately, and then combined as needed for JDK libraries, and for Hotspot. So, currently we use EXTRA_CFLAGS just to add them to CFLAGS_JDK, and then drop them. In the new Hotspot build, we use the EXTRA_CFLAGS to add them to the Hotspot flags. Of course, we could have used LEGACY_EXTRA_CFLAGS, but we'd like to avoid using "LEGACY" for new stuff. With that being said, I now realize that maybe this change will not be needed anyway, at least not right now. Since doing a proper flag cleanup is a major task, we decided to copy the CFLAGS_JDK behavior for Hotspot flags as well in the current implementation of the new hotspot build, and saving the cleanup for another day (so it will not block the new build system). So I'll revert this part of the change for now. On Windows -LD is a superset of -dll, so it isn't obvious the change is correct. I hope Erik's reply to that was satisfactory. --- jdk/make/lib/LibCommon.gmk + # Disable it here for the jdk binaries until we decide to enable them. s/binaries/libraries/ ? Sure, I'll fix. Actually both this fragment and the one in jdk/make/launcher/LauncherCommon.gmk I find confusing - what is the relation with hotspot here, and the role of SetupNativeCompilation? I hope Erik's reply to that is acceptable as well. As for the other changes in the jdk repo, most of them is to either standardize on -Wl, or to fix some places were we didn't do a proper $$ (instead of $) in an macro that was supposed to be eval'd. (This was incorrect before as well, but
Re: RFR: JDK-8142907 Integration of minor fixes from the build-infra project
Hi David, Resurrecting the second part of the build-infra integrations. :) Fortunately, no major code changes in the meantime has affected this patch. (I did need to update the new hotspot/make/lib/Lib-jdk.hotspot.agent.gmk though.) On 2015-11-16 05:33, David Holmes wrote: Hi Magnus, I had a flick through most of the files. Overall seems okay but I have a few queries below. Replies inline. On 13/11/2015 12:43 PM, Magnus Ihse Bursie wrote: The build-infra project has collected a number of minor fixes and changes during the new hotspot build development. Its a mix of code cleanup and new capabilities. Not all of these new features are immediately beneficial to the JDK, but they will be needed for the upcoming new Hotspot build, and it will not hurt to have them in mainline. (In fact, it will tremendously help merging between mainline and build-infra.) The fix addresses these issues: In general: * Break out hotspot configuration into hotspot.m4 * Long link lines uses @-files * Consistently use -Wl instead of -Xlinker * Improve clang on linux compilation * Set shared library name explicitely on solaris * Set correct shared library flag on Windows (-dll) * Consistency fixes for build toolchain * Bring compare script up to date * General code/whitespace cleanup * Additional functionality in MakeBase In NativeCompilation.gmk: * More efficient vardeps for per-file CFLAGS * Fewer shell executions (means better performance on Windows) * EXCLUDE_PATTERN and EXTRA_OBJECT_FILES * Debug symbols on macosx (disabled for existing code to keep current behavior) Enabling debug info on macosx on existing jdk should be treated in a follow-up bug. Bug: https://bugs.openjdk.java.net/browse/JDK-8142907 WebRev: http://cr.openjdk.java.net/~ihse/JDK-8142907-build-infra-integration-closed/webrev.01 (It turned out that WebRev could not at the same time include files from multiple repos and track the history of a "hg cp":ied file. So I created an alternative revision here: http://cr.openjdk.java.net/~ihse/JDK-8142907-build-infra-integration-closed/webrev.02/ It does not include the jdk files, but hotspot.m4 might be easier to understand) flags.m4: 60 AC_SUBST(LEGACY_EXTRA_CFLAGS) 61 AC_SUBST(LEGACY_EXTRA_CXXFLAGS) 62 AC_SUBST(LEGACY_EXTRA_LDFLAGS) 63 64 AC_SUBST(EXTRA_CFLAGS) 65 AC_SUBST(EXTRA_CXXFLAGS) 66 AC_SUBST(EXTRA_LDFLAGS) IIRC we added the legacy flags purely to pass the cross-compilation args through to hotspot. Not sure why we need both legacy and non-legancy variants now ?? This is part of an ongoing effort to split CFLAGS_JDK into more reasonable parts. The current model is that we throw all possible flags together in CFLAGS_JDK (and then further on to CFLAGS_JDKLIB et al). While it was questionable even before, it did work when we only built the JDK native libraries. Now that we need to build libjvm.so as well, which has (for historical reason) a set of flags that are both partially the same and partially different, this is not so good. We would like to see a change to a situation where the different "parts" that we build CFLAGS_JDK from would be handled separately, and then combined as needed for JDK libraries, and for Hotspot. So, currently we use EXTRA_CFLAGS just to add them to CFLAGS_JDK, and then drop them. In the new Hotspot build, we use the EXTRA_CFLAGS to add them to the Hotspot flags. Of course, we could have used LEGACY_EXTRA_CFLAGS, but we'd like to avoid using "LEGACY" for new stuff. With that being said, I now realize that maybe this change will not be needed anyway, at least not right now. Since doing a proper flag cleanup is a major task, we decided to copy the CFLAGS_JDK behavior for Hotspot flags as well in the current implementation of the new hotspot build, and saving the cleanup for another day (so it will not block the new build system). So I'll revert this part of the change for now. On Windows -LD is a superset of -dll, so it isn't obvious the change is correct. I hope Erik's reply to that was satisfactory. --- jdk/make/lib/LibCommon.gmk + # Disable it here for the jdk binaries until we decide to enable them. s/binaries/libraries/ ? Sure, I'll fix. Actually both this fragment and the one in jdk/make/launcher/LauncherCommon.gmk I find confusing - what is the relation with hotspot here, and the role of SetupNativeCompilation? I hope Erik's reply to that is acceptable as well. As for the other changes in the jdk repo, most of them is to either standardize on -Wl, or to fix some places were we didn't do a proper $$ (instead of $) in an macro that was supposed to be eval'd. (This was incorrect before as well, but it broke the build when the LDFLAGS started having commas in them.) /Magnus Thanks, David /Magnus
Re: RFR: JDK-8142907 Integration of minor fixes from the build-infra project
On 2015-12-11 22:43, Magnus Ihse Bursie wrote: Hi David, Resurrecting the second part of the build-infra integrations. :) Fortunately, no major code changes in the meantime has affected this patch. (I did need to update the new hotspot/make/lib/Lib-jdk.hotspot.agent.gmk though.) It seems I forgot to include the link to the new webrev: http://cr.openjdk.java.net/~ihse/JDK-8142907-build-infra-integration-closed/webrev.05 /Magnus On 2015-11-16 05:33, David Holmes wrote: Hi Magnus, I had a flick through most of the files. Overall seems okay but I have a few queries below. Replies inline. On 13/11/2015 12:43 PM, Magnus Ihse Bursie wrote: The build-infra project has collected a number of minor fixes and changes during the new hotspot build development. Its a mix of code cleanup and new capabilities. Not all of these new features are immediately beneficial to the JDK, but they will be needed for the upcoming new Hotspot build, and it will not hurt to have them in mainline. (In fact, it will tremendously help merging between mainline and build-infra.) The fix addresses these issues: In general: * Break out hotspot configuration into hotspot.m4 * Long link lines uses @-files * Consistently use -Wl instead of -Xlinker * Improve clang on linux compilation * Set shared library name explicitely on solaris * Set correct shared library flag on Windows (-dll) * Consistency fixes for build toolchain * Bring compare script up to date * General code/whitespace cleanup * Additional functionality in MakeBase In NativeCompilation.gmk: * More efficient vardeps for per-file CFLAGS * Fewer shell executions (means better performance on Windows) * EXCLUDE_PATTERN and EXTRA_OBJECT_FILES * Debug symbols on macosx (disabled for existing code to keep current behavior) Enabling debug info on macosx on existing jdk should be treated in a follow-up bug. Bug: https://bugs.openjdk.java.net/browse/JDK-8142907 WebRev: http://cr.openjdk.java.net/~ihse/JDK-8142907-build-infra-integration-closed/webrev.01 (It turned out that WebRev could not at the same time include files from multiple repos and track the history of a "hg cp":ied file. So I created an alternative revision here: http://cr.openjdk.java.net/~ihse/JDK-8142907-build-infra-integration-closed/webrev.02/ It does not include the jdk files, but hotspot.m4 might be easier to understand) flags.m4: 60 AC_SUBST(LEGACY_EXTRA_CFLAGS) 61 AC_SUBST(LEGACY_EXTRA_CXXFLAGS) 62 AC_SUBST(LEGACY_EXTRA_LDFLAGS) 63 64 AC_SUBST(EXTRA_CFLAGS) 65 AC_SUBST(EXTRA_CXXFLAGS) 66 AC_SUBST(EXTRA_LDFLAGS) IIRC we added the legacy flags purely to pass the cross-compilation args through to hotspot. Not sure why we need both legacy and non-legancy variants now ?? This is part of an ongoing effort to split CFLAGS_JDK into more reasonable parts. The current model is that we throw all possible flags together in CFLAGS_JDK (and then further on to CFLAGS_JDKLIB et al). While it was questionable even before, it did work when we only built the JDK native libraries. Now that we need to build libjvm.so as well, which has (for historical reason) a set of flags that are both partially the same and partially different, this is not so good. We would like to see a change to a situation where the different "parts" that we build CFLAGS_JDK from would be handled separately, and then combined as needed for JDK libraries, and for Hotspot. So, currently we use EXTRA_CFLAGS just to add them to CFLAGS_JDK, and then drop them. In the new Hotspot build, we use the EXTRA_CFLAGS to add them to the Hotspot flags. Of course, we could have used LEGACY_EXTRA_CFLAGS, but we'd like to avoid using "LEGACY" for new stuff. With that being said, I now realize that maybe this change will not be needed anyway, at least not right now. Since doing a proper flag cleanup is a major task, we decided to copy the CFLAGS_JDK behavior for Hotspot flags as well in the current implementation of the new hotspot build, and saving the cleanup for another day (so it will not block the new build system). So I'll revert this part of the change for now. On Windows -LD is a superset of -dll, so it isn't obvious the change is correct. I hope Erik's reply to that was satisfactory. --- jdk/make/lib/LibCommon.gmk + # Disable it here for the jdk binaries until we decide to enable them. s/binaries/libraries/ ? Sure, I'll fix. Actually both this fragment and the one in jdk/make/launcher/LauncherCommon.gmk I find confusing - what is the relation with hotspot here, and the role of SetupNativeCompilation? I hope Erik's reply to that is acceptable as well. As for the other changes in the jdk repo, most of them is to either standardize on -Wl, or to fix some places were we didn't do a proper $$ (instead of $) in an macro that was supposed to be eval'd. (This was incorrect before as well, but it broke the build when the LDFLAGS started having commas in them.) /Magnus
Re: RFR: JDK-8142907 Integration of minor fixes from the build-infra project
On 2015-11-16 05:33, David Holmes wrote: Hi Magnus, I had a flick through most of the files. Overall seems okay but I have a few queries below. On 13/11/2015 12:43 PM, Magnus Ihse Bursie wrote: The build-infra project has collected a number of minor fixes and changes during the new hotspot build development. Its a mix of code cleanup and new capabilities. Not all of these new features are immediately beneficial to the JDK, but they will be needed for the upcoming new Hotspot build, and it will not hurt to have them in mainline. (In fact, it will tremendously help merging between mainline and build-infra.) The fix addresses these issues: In general: * Break out hotspot configuration into hotspot.m4 * Long link lines uses @-files * Consistently use -Wl instead of -Xlinker * Improve clang on linux compilation * Set shared library name explicitely on solaris * Set correct shared library flag on Windows (-dll) * Consistency fixes for build toolchain * Bring compare script up to date * General code/whitespace cleanup * Additional functionality in MakeBase In NativeCompilation.gmk: * More efficient vardeps for per-file CFLAGS * Fewer shell executions (means better performance on Windows) * EXCLUDE_PATTERN and EXTRA_OBJECT_FILES * Debug symbols on macosx (disabled for existing code to keep current behavior) Enabling debug info on macosx on existing jdk should be treated in a follow-up bug. Bug: https://bugs.openjdk.java.net/browse/JDK-8142907 WebRev: http://cr.openjdk.java.net/~ihse/JDK-8142907-build-infra-integration-closed/webrev.01 (It turned out that WebRev could not at the same time include files from multiple repos and track the history of a "hg cp":ied file. So I created an alternative revision here: http://cr.openjdk.java.net/~ihse/JDK-8142907-build-infra-integration-closed/webrev.02/ It does not include the jdk files, but hotspot.m4 might be easier to understand) flags.m4: 60 AC_SUBST(LEGACY_EXTRA_CFLAGS) 61 AC_SUBST(LEGACY_EXTRA_CXXFLAGS) 62 AC_SUBST(LEGACY_EXTRA_LDFLAGS) 63 64 AC_SUBST(EXTRA_CFLAGS) 65 AC_SUBST(EXTRA_CXXFLAGS) 66 AC_SUBST(EXTRA_LDFLAGS) IIRC we added the legacy flags purely to pass the cross-compilation args through to hotspot. Not sure why we need both legacy and non-legancy variants now ?? This actually looks weird to me too. I can find no usage of EXTRA_CFLAGS without the LEGACY_ prefix. On Windows -LD is a superset of -dll, so it isn't obvious the change is correct. It seems, /LD is a compiler option and /DLL is a linker option. Setting /LD to cl.exe makes it set /DLL to link.exe. Since we link using link.exe we must use -dll. Looking closer at the change, we are actually setting SHARED_LIBRARY_FLAGS=-DL on windows, but then never using that variable. Instead hard coding -dll further down. This change corrects SHARED_LIBRARY_FLAGS to -dll and then uses that value. --- jdk/make/lib/LibCommon.gmk + # Disable it here for the jdk binaries until we decide to enable them. s/binaries/libraries/ ? Actually both this fragment and the one in jdk/make/launcher/LauncherCommon.gmk I find confusing - what is the relation with hotspot here, and the role of SetupNativeCompilation? In SetupNativeCompilation, we currently disable debug symbols creation on Macosx. Back in JDK 8 when we converted the jdk build to build-infra, that was how the old build worked and we just mimicked. Until now, we haven't gotten around to fixing debug symbols support for macosx. In the build-infra forest, since debug symbols are needed for hotspot, we have implemented support in SetupNativeCompilation. Magnus intends to bring this support into JDK 9 with this change, the motivation being to reduce differences between build-infra and JDK 9 which are complicating merging. We think actually enabling debug symbols support for the JDK libraries should rather be a separate change, that we want to follow up with soon after. /Erik Thanks, David /Magnus
Re: RFR: JDK-8142907 Integration of minor fixes from the build-infra project
Hi Magnus, I had a flick through most of the files. Overall seems okay but I have a few queries below. On 13/11/2015 12:43 PM, Magnus Ihse Bursie wrote: The build-infra project has collected a number of minor fixes and changes during the new hotspot build development. Its a mix of code cleanup and new capabilities. Not all of these new features are immediately beneficial to the JDK, but they will be needed for the upcoming new Hotspot build, and it will not hurt to have them in mainline. (In fact, it will tremendously help merging between mainline and build-infra.) The fix addresses these issues: In general: * Break out hotspot configuration into hotspot.m4 * Long link lines uses @-files * Consistently use -Wl instead of -Xlinker * Improve clang on linux compilation * Set shared library name explicitely on solaris * Set correct shared library flag on Windows (-dll) * Consistency fixes for build toolchain * Bring compare script up to date * General code/whitespace cleanup * Additional functionality in MakeBase In NativeCompilation.gmk: * More efficient vardeps for per-file CFLAGS * Fewer shell executions (means better performance on Windows) * EXCLUDE_PATTERN and EXTRA_OBJECT_FILES * Debug symbols on macosx (disabled for existing code to keep current behavior) Enabling debug info on macosx on existing jdk should be treated in a follow-up bug. Bug: https://bugs.openjdk.java.net/browse/JDK-8142907 WebRev: http://cr.openjdk.java.net/~ihse/JDK-8142907-build-infra-integration-closed/webrev.01 (It turned out that WebRev could not at the same time include files from multiple repos and track the history of a "hg cp":ied file. So I created an alternative revision here: http://cr.openjdk.java.net/~ihse/JDK-8142907-build-infra-integration-closed/webrev.02/ It does not include the jdk files, but hotspot.m4 might be easier to understand) flags.m4: 60 AC_SUBST(LEGACY_EXTRA_CFLAGS) 61 AC_SUBST(LEGACY_EXTRA_CXXFLAGS) 62 AC_SUBST(LEGACY_EXTRA_LDFLAGS) 63 64 AC_SUBST(EXTRA_CFLAGS) 65 AC_SUBST(EXTRA_CXXFLAGS) 66 AC_SUBST(EXTRA_LDFLAGS) IIRC we added the legacy flags purely to pass the cross-compilation args through to hotspot. Not sure why we need both legacy and non-legancy variants now ?? On Windows -LD is a superset of -dll, so it isn't obvious the change is correct. --- jdk/make/lib/LibCommon.gmk + # Disable it here for the jdk binaries until we decide to enable them. s/binaries/libraries/ ? Actually both this fragment and the one in jdk/make/launcher/LauncherCommon.gmk I find confusing - what is the relation with hotspot here, and the role of SetupNativeCompilation? Thanks, David /Magnus
RFR: JDK-8142907 Integration of minor fixes from the build-infra project
The build-infra project has collected a number of minor fixes and changes during the new hotspot build development. Its a mix of code cleanup and new capabilities. Not all of these new features are immediately beneficial to the JDK, but they will be needed for the upcoming new Hotspot build, and it will not hurt to have them in mainline. (In fact, it will tremendously help merging between mainline and build-infra.) The fix addresses these issues: In general: * Break out hotspot configuration into hotspot.m4 * Long link lines uses @-files * Consistently use -Wl instead of -Xlinker * Improve clang on linux compilation * Set shared library name explicitely on solaris * Set correct shared library flag on Windows (-dll) * Consistency fixes for build toolchain * Bring compare script up to date * General code/whitespace cleanup * Additional functionality in MakeBase In NativeCompilation.gmk: * More efficient vardeps for per-file CFLAGS * Fewer shell executions (means better performance on Windows) * EXCLUDE_PATTERN and EXTRA_OBJECT_FILES * Debug symbols on macosx (disabled for existing code to keep current behavior) Enabling debug info on macosx on existing jdk should be treated in a follow-up bug. Bug: https://bugs.openjdk.java.net/browse/JDK-8142907 WebRev: http://cr.openjdk.java.net/~ihse/JDK-8142907-build-infra-integration-closed/webrev.01 (It turned out that WebRev could not at the same time include files from multiple repos and track the history of a "hg cp":ied file. So I created an alternative revision here: http://cr.openjdk.java.net/~ihse/JDK-8142907-build-infra-integration-closed/webrev.02/ It does not include the jdk files, but hotspot.m4 might be easier to understand) /Magnus
Re: RFR: JDK-8142907 Integration of minor fixes from the build-infra project
Looks good. /Erik On 2015-11-13 03:43, Magnus Ihse Bursie wrote: The build-infra project has collected a number of minor fixes and changes during the new hotspot build development. Its a mix of code cleanup and new capabilities. Not all of these new features are immediately beneficial to the JDK, but they will be needed for the upcoming new Hotspot build, and it will not hurt to have them in mainline. (In fact, it will tremendously help merging between mainline and build-infra.) The fix addresses these issues: In general: * Break out hotspot configuration into hotspot.m4 * Long link lines uses @-files * Consistently use -Wl instead of -Xlinker * Improve clang on linux compilation * Set shared library name explicitely on solaris * Set correct shared library flag on Windows (-dll) * Consistency fixes for build toolchain * Bring compare script up to date * General code/whitespace cleanup * Additional functionality in MakeBase In NativeCompilation.gmk: * More efficient vardeps for per-file CFLAGS * Fewer shell executions (means better performance on Windows) * EXCLUDE_PATTERN and EXTRA_OBJECT_FILES * Debug symbols on macosx (disabled for existing code to keep current behavior) Enabling debug info on macosx on existing jdk should be treated in a follow-up bug. Bug: https://bugs.openjdk.java.net/browse/JDK-8142907 WebRev: http://cr.openjdk.java.net/~ihse/JDK-8142907-build-infra-integration-closed/webrev.01 (It turned out that WebRev could not at the same time include files from multiple repos and track the history of a "hg cp":ied file. So I created an alternative revision here: http://cr.openjdk.java.net/~ihse/JDK-8142907-build-infra-integration-closed/webrev.02/ It does not include the jdk files, but hotspot.m4 might be easier to understand) /Magnus
Re: RFR: JDK-8142907 Integration of minor fixes from the build-infra project
On 2015-11-13 12:07, Magnus Ihse Bursie wrote: It turned out I was a bit to quick to send out this review request. There were a couple of issues I didn't spot earlier. The new -h flag on solaris prompted the linker to complain when we tried to set a soname on an executable. We've done this for other toolchains before but they have just silently ignored our error. The changes in link flags to include a comma highlighted some places were $ was used instead of $$ when calling SetupNativeCompilation within another macro (jdk launchers and demos). And finally, Erik reported to me privately that there was dead code in hotspot.m4 regarding SALIB_NAME, which is not used in the old hotspot build nor will be used in the new, so I removed that as well. New, updated review: http://cr.openjdk.java.net/~ihse/JDK-8142907-build-infra-integration-closed/webrev.03/ After discussion with Erik, I have also removed the following section from compare_exceptions.sh.incl: # Some xor instructions end up with different args in the lib but not in the object files. ACCEPTED_DIS_DIFF=" ./demo/jvmti/waiters/lib/libwaiters.so With the improved DIS_DIFF_FILTER, this exception was not needed. Sorry about this -- the build-infra and the mainline version of the exception list file had drifted quite far away from each other, making the merge horrible :-& /Magnus /Magnus On 2015-11-13 03:43, Magnus Ihse Bursie wrote: The build-infra project has collected a number of minor fixes and changes during the new hotspot build development. Its a mix of code cleanup and new capabilities. Not all of these new features are immediately beneficial to the JDK, but they will be needed for the upcoming new Hotspot build, and it will not hurt to have them in mainline. (In fact, it will tremendously help merging between mainline and build-infra.) The fix addresses these issues: In general: * Break out hotspot configuration into hotspot.m4 * Long link lines uses @-files * Consistently use -Wl instead of -Xlinker * Improve clang on linux compilation * Set shared library name explicitely on solaris * Set correct shared library flag on Windows (-dll) * Consistency fixes for build toolchain * Bring compare script up to date * General code/whitespace cleanup * Additional functionality in MakeBase In NativeCompilation.gmk: * More efficient vardeps for per-file CFLAGS * Fewer shell executions (means better performance on Windows) * EXCLUDE_PATTERN and EXTRA_OBJECT_FILES * Debug symbols on macosx (disabled for existing code to keep current behavior) Enabling debug info on macosx on existing jdk should be treated in a follow-up bug. Bug: https://bugs.openjdk.java.net/browse/JDK-8142907 WebRev: http://cr.openjdk.java.net/~ihse/JDK-8142907-build-infra-integration-closed/webrev.01 (It turned out that WebRev could not at the same time include files from multiple repos and track the history of a "hg cp":ied file. So I created an alternative revision here: http://cr.openjdk.java.net/~ihse/JDK-8142907-build-infra-integration-closed/webrev.02/ It does not include the jdk files, but hotspot.m4 might be easier to understand) /Magnus
Re: RFR: JDK-8142907 Integration of minor fixes from the build-infra project
It turned out I was a bit to quick to send out this review request. There were a couple of issues I didn't spot earlier. The new -h flag on solaris prompted the linker to complain when we tried to set a soname on an executable. We've done this for other toolchains before but they have just silently ignored our error. The changes in link flags to include a comma highlighted some places were $ was used instead of $$ when calling SetupNativeCompilation within another macro (jdk launchers and demos). And finally, Erik reported to me privately that there was dead code in hotspot.m4 regarding SALIB_NAME, which is not used in the old hotspot build nor will be used in the new, so I removed that as well. New, updated review: http://cr.openjdk.java.net/~ihse/JDK-8142907-build-infra-integration-closed/webrev.03/ /Magnus On 2015-11-13 03:43, Magnus Ihse Bursie wrote: The build-infra project has collected a number of minor fixes and changes during the new hotspot build development. Its a mix of code cleanup and new capabilities. Not all of these new features are immediately beneficial to the JDK, but they will be needed for the upcoming new Hotspot build, and it will not hurt to have them in mainline. (In fact, it will tremendously help merging between mainline and build-infra.) The fix addresses these issues: In general: * Break out hotspot configuration into hotspot.m4 * Long link lines uses @-files * Consistently use -Wl instead of -Xlinker * Improve clang on linux compilation * Set shared library name explicitely on solaris * Set correct shared library flag on Windows (-dll) * Consistency fixes for build toolchain * Bring compare script up to date * General code/whitespace cleanup * Additional functionality in MakeBase In NativeCompilation.gmk: * More efficient vardeps for per-file CFLAGS * Fewer shell executions (means better performance on Windows) * EXCLUDE_PATTERN and EXTRA_OBJECT_FILES * Debug symbols on macosx (disabled for existing code to keep current behavior) Enabling debug info on macosx on existing jdk should be treated in a follow-up bug. Bug: https://bugs.openjdk.java.net/browse/JDK-8142907 WebRev: http://cr.openjdk.java.net/~ihse/JDK-8142907-build-infra-integration-closed/webrev.01 (It turned out that WebRev could not at the same time include files from multiple repos and track the history of a "hg cp":ied file. So I created an alternative revision here: http://cr.openjdk.java.net/~ihse/JDK-8142907-build-infra-integration-closed/webrev.02/ It does not include the jdk files, but hotspot.m4 might be easier to understand) /Magnus
