RFR: JDK-8142907 Integration of minor fixes from the build-infra project

[David Holmes]

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. It's 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
>>>>
>>>
>