RFR (S): JDK-8142909 Integration of minor fixes from the build-infra project

[David Holmes]

On 16/11/2015 6:22 PM, Erik Joelsson wrote:
>
>
> On 2015-11-16 03:10, David Holmes wrote:
>> Hi Magnus,
>>
>> On 13/11/2015 9:33 PM, Magnus Ihse Bursie wrote:
>>> On 2015-11-13 09:13, Erik Joelsson wrote:
>>>> Hello,
>>>>
>>>> make/bsd/makefiles/amd64.make:
>>>> make/bsd/makefiles/gcc.make:
>>>> Perhaps the order of "$(OPT_CFLAGS/NOOPT) $(OPT_CFLAGS/$(BUILDARCH))"
>>>> should be reversed to guarantee that NOOPT is the one used in case
>>>> BUILDARCH contains something that conflicts? The solaris file does it
>>>> this way.
>>> You know you wrote that code originally? ;-)
>>>
>>> Yeah, I agree, it's more reasonable. Updated webrev:
>>> http://cr.openjdk.java.net/~ihse/JDK-8142909-hotspot-build-infra-integration/webrev.02
>>>
>>
>> I don't like this particular change as it is too generic. It gives the
>> appearance of being apply to apply whatever the BUILDARCH specific
>> optflags are, which makes no sense if the whole point here is to not
>> optimize these files. If all this is intended to do is include -g then
>> I think that should be done explicitly.
>>
> The -g flag is added to OPT_CFLAGS/$(BUILDARCH) and I can only assume
> the intention was to have it added to all compilation command lines.
> However, the OPT_CFLAGS/<filename> variable overrides
> CFLAGS/$(BUILDARCH), effectively removing the -g flag from the files
> where we explicitly change the optimization level. The patch intends to
> make sure -g is used on those files too.

I understand that, but it is far from obvious that OPT_FLAGS/BUILDARCH 
means -g (why???), or that putting other things in that variable might 
break files that should not be optimized.

> A cleaner solution would be to not have -g be part of the OPT flags at
> all but its own set of DEBUG_CFLAGS.

I thought -g was in the DEBUG_CFLAGS ?? Or maybe it used to be. Anyway 
if all we are interested in is adding -g then I agree it should have its 
own variable.

>> ---
>>
>> make/bsd/makefiles/saproc.make
>>
>> ! # Order src files alfabetically
>>
>> That would be "alphabetically". But that list doesn't seem alphabetic
>> anyway: MacosxDebuggerLocal.m would come after libproc_impl.c; and
>> $(AGENTDIR) should come first. I really see no point in forcing such
>> lists to be in alphabetic order.
>>
> This is to make comparison of binaries between the old hotspot build and
> the new hotspot build easier. In build-infra, we $(sort ) the object
> files before linking to get a reproducible order. The linker output is
> affected by the order of the object files. By making sure the old and
> the new build sorts them the same way, we get cleaner comparisons and
> can more easily detect other types of differences through those
> comparisons. The sort order isn't strictly alphabetical, it's on byte
> order so upper case goes before lower case.

I saw the change for the ordered object files on Windows, but this 
simply orders source files in a list. Not sure how that has any affect 
on the linking ??

Cheers,
David

> /Erik
>> make/linux/makefiles/saproc.make doesn't have the comment that was
>> added for bsd.
>>
>> ----
>>
>> make/solaris/makefiles/amd64.make
>>
>> Similar issue with bsd amd64.make. Not sure what you are trying to
>> achieve here - is it some kind of last option wins situation ?
>>
>> ---
>>
>> make/windows/create_obj_files.sh
>>
>> Harmless I guess but not sure about relevance of sort order here.
>>
>> ----
>>
>>  src/share/vm/adlc/adlparse.cpp
>>
>> So that's where that comes from! :)
>>
>> ---
>>
>> src/share/vm/gc/g1/g1EvacStats.cpp
>>
>> I don't see anything in the cpp files that uses anything from the
>> atomic class ???
>>
>>
>> Thanks,
>> David
>>
>>
>>>
>>> /Magnus
>>>
>>>>
>>>> Otherwise looks good.
>>>>
>>>> /Erik
>>>>
>>>> On 2015-11-13 03:34, Magnus Ihse Bursie wrote:
>>>>> In the new hotspot build project, we have made a few changes to the
>>>>> existing build. In preparation for the new build, I'd like to
>>>>> integrate these into mainline.
>>>>>
>>>>> These changes are:
>>>>> * When overriding optimization, do not lose current debug (-g)
>>>>> setting (!)
>>>>> * Make adlc actually quiet in quiet mode
>>>>> * Make g1EvacStats.cpp compile in all cases without precompiled
>>>>> headers
>>>>> * Sort saproc object files when linking (facilitates comparison to
>>>>> new build)
>>>>>
>>>>> Unless someone suggests otherwise, I intend to push this (using JPRT)
>>>>> to hs-rt.
>>>>>
>>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8142909
>>>>> WebRev:
>>>>> http://cr.openjdk.java.net/~ihse/JDK-8142909-hotspot-build-infra-integration/webrev.01
>>>>>
>>>>>
>>>>>
>>>>> /Magnus
>>>>
>>>
>