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

[Erik Joelsson]

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.

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

/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
>>>
>>