[7u60] RFR 8024648 : 8016131 breaks Zero port

David Holmes david.holmes at oracle.com
Fri Feb 7 12:53:58 PST 2014


On 8/02/2014 2:11 AM, Andrew Hughes wrote:
> ----- Original Message -----
>> I have a concern here.
>>
>> First this general situation is a mess. We started adding the new-build
>> support in hs24 but JDK 8 is using hs25, so all this SPEC and
>> JVM_VARIANT stuff is completely unwanted in JDK 7u. My preference here
>> would be to get this stuff backed out of 7u and return to a completely
>> configure-ignorant build system. That said I'm not sure that is really
>> the issue with ZERO/SHARK builds.
>>
>
> Backing out:
>
> 7141246: build-infra merge: Introduce new JVM_VARIANT* to control which kind of jvm gets built
> Reviewed-by: dholmes, ohair
>
> would also work, or at least the Zero/Shark changes there. I note you
> reviewed this, but it clearly wasn't tested with Zero. With this change
> introduced, a Zero build can't even find its makefiles any more.

We don't/can't test zero/shark. The situation has always been that the 
zero/shark communities need to check any changes and come back to us 
with patches when needed. Some engineers have been able to devote their 
own time to aiding in this area.

Hopefully you saw my follow-up email with a proposed simpler fix.

Thanks,
David

>> My concern with the proposed patch is that defs.make is not intended to
>> be included by every other Makefile and/or .make file. We all know by
>> now that the hotspot build system is complex, convoluted and fragile.
>> When you do a hotspot build there are multiple nested sub-makes
>> involved, as well as build file generation and use. The "big picture"
>> intent is that defs.make contains all the high-level stuff needed by the
>> "top" level makefile targets. buildtree.make will then generate other
>> .make files using that information. It is very difficult to see exactly
>> which .make file will get used by which phase of the overall build.
>> Unfortunately this leads to a tendency where if a "low-level" file needs
>> access to X, which is defined in defs.make, then the "solution" is just
>> to include defs.make in that file. We did the same thing for
>> buildtree.make in JDK 8. But I have reservations about also doing it for
>> gcc.make and vm.make. I think it is the wrong solution - we should be
>> passing the needed variables/values through the files generated by
>> buildtree.make.
>>
>
> Each change was introduced separately in IcedTea, as they were caught at
> different times.
>
> changeset:   4224:a57f19258524
> user:        andrew
> date:        Fri Jan 04 22:21:54 2013 +0000
> summary:     Include defs.make in buildtree.make so ZERO_BUILD is recognised and JVM_VARIANT_ZERO set.
>
> changeset:   4894:56f4e181b8d4
> user:        andrew
> date:        Wed Sep 11 16:55:12 2013 +0100
> summary:     Set ZERO_BUILD in flags.make so it is set on rebuilds
>
> changeset:   4971:02f0bce4ac32
> user:        andrew
> date:        Tue Nov 12 17:58:35 2013 +0000
> summary:     Include defs.make in vm.make so VM_LITTLE_ENDIAN is defined on Zero builds
>
> I agree it may not be the best solution for all, which
> is why I only fixed buildtree.make at first, where it's most obviously
> wrong. However, I don't know the makefiles well enough to suggest another
> solution and just adding defs.make does work for us. We did have another
> solution for gcc.make, at least for the Zero part:
>
> changeset:   4279:4fd8e7ebca9a
> parent:      4260:7e12b7098f20
> user:        andrew
> date:        Thu Jan 17 12:07:28 2013 +0000
> summary:     Make sure libffi cflags and libs are used.
>
> diff -r 7e12b7098f20 -r 4fd8e7ebca9a make/linux/makefiles/gcc.make
> --- a/make/linux/makefiles/gcc.make	Mon Jan 14 16:38:37 2013 +0000
> +++ b/make/linux/makefiles/gcc.make	Thu Jan 17 12:07:28 2013 +0000
> @@ -75,9 +75,6 @@
>   VM_PICFLAG/AOUT   =
>   VM_PICFLAG        = $(VM_PICFLAG/$(LINK_INTO))
>
> -ifeq ($(JVM_VARIANT_ZERO), true)
> -CFLAGS += $(LIBFFI_CFLAGS)
> -endif
>   ifeq ($(JVM_VARIANT_ZEROSHARK), true)
>   CFLAGS += $(LIBFFI_CFLAGS)
>   CFLAGS += $(LLVM_CFLAGS)
> diff -r 7e12b7098f20 -r 4fd8e7ebca9a make/linux/makefiles/vm.make
> --- a/make/linux/makefiles/vm.make	Mon Jan 14 16:38:37 2013 +0000
> +++ b/make/linux/makefiles/vm.make	Thu Jan 17 12:07:28 2013 +0000
> @@ -284,9 +284,6 @@
>
>     LIBS_VM                  += $(LIBS)
>   endif
> -ifeq ($(JVM_VARIANT_ZERO), true)
> -  LIBS_VM += $(LIBFFI_LIBS)
> -endif
>   ifeq ($(JVM_VARIANT_ZEROSHARK), true)
>     LIBS_VM   += $(LIBFFI_LIBS) $(LLVM_LIBS)
>     LFLAGS_VM += $(LLVM_LDFLAGS)
> diff -r 7e12b7098f20 -r 4fd8e7ebca9a make/linux/makefiles/zero.make
> --- a/make/linux/makefiles/zero.make	Mon Jan 14 16:38:37 2013 +0000
> +++ b/make/linux/makefiles/zero.make	Thu Jan 17 12:07:28 2013 +0000
> @@ -30,3 +30,7 @@
>
>   # Install libjvm.so, etc in in server directory.
>   VM_SUBDIR = server
> +
> +# Make sure libffi is included
> +CFLAGS += $(LIBFFI_CFLAGS)
> +LIBS_VM += $(LIBFFI_LIBS)
>
> That leaves Shark still broken but maybe something similar could be done
> with the LLVM flags instead. I haven't tested Shark with this patch and
> that's much more involved. I've also never had a working build of it
> anyway.
>
>> If this was to go ahead as-is I would want to know that there has been a
>> comparison of all the build commands before and after to ensure no
>> unintended changes have been introduced. A set of successful builds is
>> not in itself sufficient.
>
> Assuming you run jdk_generic_profile.sh first, setting ZERO_BUILD=true
> is sufficient to build Zero.
>
>>
>> David
>>
>> On 7/02/2014 8:10 AM, Alejandro E Murillo wrote:
>>> I  forgot to change the "subject".
>>> The change is and has been approved for 7u60 (hs24.60)
>>> Thanks
>>>
>>> Alejandro
>>>
>>> On 2/6/2014 3:03 PM, Alejandro E Murillo wrote:
>>>>
>>>> can we get 2  hotspot reviewers quickly  check this change ?
>>>> thanks
>>>> Alejandro
>>>>
>>>> -------- Original Message --------
>>>> Subject:     Re: Question on 8024648 : 8016131 breaks Zero port
>>>> Date:     Thu, 6 Feb 2014 16:21:29 -0500 (EST)
>>>> From:     Andrew Hughes <gnu.andrew at redhat.com>
>>>> To:     Alejandro E Murillo <alejandro.murillo at oracle.com>
>>>> CC:     Seán Coffey <sean.coffey at oracle.com>, Omair Majid
>>>> <omajid at redhat.com>, jdk7u-dev at openjdk.java.net
>>>>
>>>>
>>>>
>>>> ----- Original Message -----
>>>>
>>>>
>>>> Here's the webrev:
>>>>
>>>> http://cr.openjdk.java.net/~andrew/jdk7u/8024648/
>>>>
>>>> My OpenJDK username is andrew (should already be included in the commit)
>>>>
>>>> The changes are as follows:
>>>>
>>>> * Bring in defs.make to a number of makefiles which expect
>>>> JVM_VARIANT_ZERO
>>>> or JVM_VARIANT_ZEROSHARK to be set, but don't parse the makefile that
>>>> sets
>>>> it. This is a regression caused by the new build which instead sets these
>>>> via configure and $(SPEC). There is code for handling the old build
>>>> (ZERO_BUILD=true) in defs.make but it isn't brought in. (see 7141246)
>>>> * Update the call_wrapper function following 8024648.
>>>>
>>>> I've updated the bug summary to reflect that it fixes changes from both
>>>> bugs, but we can split them into two separate bugs if you prefer. Without
>>>> the 7141246 fix, the build fails in seconds.
>>>>
>>>> With the webrev applied:
>>>>
>>>> $ /mnt/builder/hsx-jdk7u/j2sdk-image/bin/java -version
>>>> openjdk version "1.7.0-internal"
>>>> OpenJDK Runtime Environment (build
>>>> 1.7.0-internal-andrew_2014_02_06_19_43-b00)
>>>> OpenJDK 64-Bit Zero VM (build 24.80-b01, interpreted mode)
>>>>
>>>> $ /mnt/builder/hsx-jdk7u/j2sdk-image/bin/java HelloWorld
>>>> Hello World
>>>>
>>>>
>>>
>>
>


More information about the hotspot-dev mailing list