RFR (M): 8024265: Enable new build on AIX (top level part)
Volker Simonis
volker.simonis at gmail.com
Wed Sep 11 09:45:35 PDT 2013
Argh! It conflicts with 8020411
(http://cr.openjdk.java.net/~erikj/8020411/webrev.root.01/) from your
last jdk8->stage synchronisation.
@Magnus, Erik : it seems that '8020411' needed a similar 'feature' to
my actual change but did it without abstracting over the name of the
"compilers target bits" flag name vs. its actual value. Unfortunately
all the users of the change 8020411 are in the closed sources. I'd
really like to stay with my solution (because I think that's the most
general one) and resolve the merge conflicts with 8020411 by
eliminating TARGET_BITS_FLAG which can now be replaced by
"${COMPILER_TARGET_BITS_FLAG}${OPENJDK_TARGET_CPU_BITS}.
But for this I'd need your review and your help to update the closed
sources accordingly (i.e all places which currently use
"${TARGET_BITS_FLAG}" would have to be changed to use
"${COMPILER_TARGET_BITS_FLAG}${OPENJDK_TARGET_CPU_BITS}" instead).
Here's my new webrev, which should cleanly apply to
http://hg.openjdk.java.net/ppc-aix-port/stage - including
generated-configure.sh :)
http://cr.openjdk.java.net/~simonis/webrevs/8024265.v4/
Thank you and best regards,
Volker
Could sombody please
On Tue, Sep 10, 2013 at 8:44 PM, Vladimir Kozlov
<vladimir.kozlov at oracle.com> wrote:
>
> Volker,
>
> I can't apply these changes to http://hg.openjdk.java.net/ppc-aix-port/stage :
>
> patching file common/autoconf/help.m4
> Hunk #4 succeeded at 83 with fuzz 1 (offset 0 lines).
> Hunk #5 succeeded at 104 with fuzz 1 (offset 0 lines).
> patching file common/autoconf/platform.m4
> Hunk #2 FAILED at 414
> 1 out of 4 hunks FAILED -- saving rejects to file common/autoconf/platform.m4.rej
> patching file common/autoconf/spec.gmk.in
> Hunk #1 FAILED at 304
> 1 out of 1 hunks FAILED -- saving rejects to file common/autoconf/spec.gmk.in.rej
> patching file common/autoconf/toolchain.m4
> Hunk #11 succeeded at 983 with fuzz 2 (offset 0 lines).
> abort: patch failed to apply
>
> Vladimir
>
>
> On 9/10/13 6:22 AM, Volker Simonis wrote:
>>
>>
>>
>>
>> On Tue, Sep 10, 2013 at 3:05 PM, Magnus Ihse Bursie
>> <magnus.ihse.bursie at oracle.com <mailto:magnus.ihse.bursie at oracle.com>>
>>
>> wrote:
>>
>> Sorry, I missed your question. I can't speak for the aix-port repo,
>> but I'm satisfied from the build point of view.
>>
>>
>> Thanks. I've added you as reviewer.
>>
>> /Magnus
>>
>> 10 sep 2013 kl. 14:22 skrev Volker Simonis <volker.simonis at gmail.com
>> <mailto:volker.simonis at gmail.com>>:
>>
>>>
>>>
>>>
>>> On Mon, Sep 9, 2013 at 4:32 PM, Volker Simonis
>>> <volker.simonis at gmail.com <mailto:volker.simonis at gmail.com>> wrote:
>>>
>>> Hi Magnus,
>>>
>>> thanks again for the review. Please see my comments inline:
>>>
>>>
>>> On Mon, Sep 9, 2013 at 12:23 PM, Magnus Ihse Bursie
>>> <magnus.ihse.bursie at oracle.com
>>> <mailto:magnus.ihse.bursie at oracle.com>> wrote:
>>>
>>> Hi Volker,
>>>
>>> Some more comments inlined.
>>>
>>>> OK, common/autoconf/build-aux/autoconf-config.guess was
>>>> too old and didn't knew about AIX 7 so it returned the
>>>> default AIX fallback which is 'rs6000-ibm-aix'. I have
>>>> now fixed 'autoconf-config.guess' to know about AIX 7 (a
>>>> one character change which is already in autoconf-2.69 so
>>>> we won't have problems if you should ever update
>>>> autoconf-config.guess).
>>>
>>>
>>> Hmm... While more elegant, the idea was that
>>> autoconf-config.guess should be a strict copy of the
>>> config.guess file from the autoconf package. The whole
>>> idea of wrapping it in a custom config.guess was to avoid
>>> "forking" the autoconf config.guess, with small changes
>>> (like this) that would be hard to track and might get lost
>>> if we update to a newer version from the upstream autoconf
>>> file.
>>>
>>> That being said, since we settled on autoconf 2.69, we
>>> really should update the file to config.guess from
>>> autoconf-2.69. I've started a test run on our internal
>>> test system with the config.guess from 2.69 to see if it
>>> breaks any existing platforms. If not, I suggest we do a
>>> fast integration of the new config.guess.
>>>
>>> Can you hold on with your changes until a new config.guess
>>> comes in? Even if you're just making a small change that
>>> will be reverted soon after, I think it would set a
>>> unfortunate precedent.
>>>
>>>
>>> That's OK for me. Actually my change still works without the
>>> changes in "config.guess" on AIX 5.3. And it will
>>> automatically start working once we get the new "config.guess".
>>>
>>>>
>>>> Ahrgh, all these proud compilers with their own ways
>>>> of expressing the same functionality. :( I assume
>>>> that you are using the COMP_MODE_OPTION in the jdk
>>>> projct? I couldn't find any references to it in the
>>>> Hotspot build changes, and otherwise there seems to
>>>> be no reason to export it in the spec.gmk file.
>>>>
>>>>
>>>> Yes, exactly. It is used in
>>>> 'jdk/makefiles/GensrcX11Wrappers.gmk'
>>>> (MEMORY_MODEL_FLAG="$(COMP_MODE_OPTION)$*").
>>>
>>> Once again the X11 wrappers. We should really make an
>>> effort and get rid of them. :-/
>>>
>>>
>>> I don't mind:)
>>>
>>>> I fully agree with the criticism on the name:) After we
>>>> already have 'COMPILER_SUPPORTS_TARGET_BITS_FLAG' I've
>>>> simply renamed it to 'COMPILER_TARGET_BITS_FLAG'. I think
>>>> that's much more appropriate and if you don't like it we
>>>> should ask the one who invented
>>>> 'COMPILER_SUPPORTS_TARGET_BITS_FLAG' :) And I had to set
>>>> 'COMPILER_TARGET_BITS_FLAG' a little earlier such that
>>>> is availabel in PLATFORM_SET_COMPILER_TARGET_BITS_FLAGS.
>>>
>>> Sounds good! Nice aligning with the existing macro.
>>>
>>> However, you forgot to change the name in spec.gmk.in
>>> <http://spec.gmk.in>. (At least in the webrev you published.)
>>>
>>>
>>> Good catch! I didn't realize it because the build in the stage
>>> repositories currently doesn’t reach the X11 wrappers.
>>> I've changed it and moved it near the definition of
>>> 'COMPILER_SUPPORTS_TARGET_BITS_FLAG'
>>>
>>>
>>>> 2. After you pointed out that setting '-q64' as extra
>>>> flags on the configure command line is not the way it is
>>>> supposed to work I recalled that we also have this
>>>> problem on older Linux/PPC64 boxes (e.g. SLES 10) where
>>>> the default compiler produces 32-bit objects by default.
>>>> To fix this problem as well, I've inserted a call to
>>>> PLATFORM_SET_COMPILER_TARGET_BITS_FLAGS followed by a
>>>> second call to AC_CHECK_SIZEOF([int *], [1111]) in the
>>>> case where we would otherwise have bailed out because the
>>>> "TESTED_TARGET_CPU_BITS" differs from the actual
>>>> "OPENJDK_TARGET_CPU_BITS". I think this change should not
>>>> affect any existing platforms because it will only be
>>>> triggered where we woould have bailed out with an error
>>>> anyway.
>>>>
>>>> Also, the workaround for autoconf bug
>>>> http://lists.gnu.org/archive/html/autoconf/2010-07/msg00004.html
>>>> in AC_CHECK_SIZEOF isn't needed any more now that we
>>>> require at least autoconf-2.69 because the problem was
>>>> fixed in 2.67. And if I looked at it more carefully I
>>>> must say that I don't understand the workaround at all.
>>>> In my opinion, the test "x$SIZEOF_INT_P" !=
>>>> "x$ac_cv_sizeof_int_p" will always fail, because the
>>>> AC_CHECK_SIZEOF macro only writes a define for
>>>> SIZEOF_INT_P into "confdefs.h" (as can be seen in
>>>> generated-configure.h) but it never defines it in the
>>>> shell. And defining SIZEOF_INT_P in the configure shell
>>>> script wouldn't help if the define written by the
>>>> AC_CHECK_SIZEOF macro was wrong (as described in the
>>>> bug). So Ithink the best is to remove the workaround and
>>>> use "ac_cv_sizeof_int_p" in the places where we used
>>>> AC_CHECK_SIZEOF before.
>>>
>>>
>>> Good to get rid of the old workaround. I agree, it looks
>>> kind of weird. I think I might have been behind some of
>>> the weirdness; I think I interpreted the autoconf
>>> documentation as if it should assign a variable
>>> SIZEOF_INT_P in the configure script, and that the $ac_*
>>> variables were internal variables that should not be
>>> directly accessed. In the current documentation, at least,
>>> the $ac_cv_sizeof* macro is officially mentioned so it
>>> should be safe to use.
>>>
>>> However, your second relies on some internal autoconf
>>> magic, by unsetting variables and defines. We've tried to
>>> avoid that, but at times there were no choice. Since we're
>>> about to fail anyway, and the code is more in place for
>>> future, strange platforms, it's probably no harm.
>>>
>>>
>>> Exactly.
>>>
>>> Is it OK if I push it now to
>>> http://hg.openjdk.java.net/ppc-aix-port/ppc-aix-port/stage or
>>> is there anything you want to test first?
>>>
>>>
>>> Vladimir will push the changes because he also needs to
>>> re-generate the closed config files (thanks Vladimir).
>>>
>>> Here's the final webrev:
>>>
>>> http://cr.openjdk.java.net/~simonis/webrevs/8024265.v3
>>>
>>> @Vladimir: I used autoconf 2.69 to create the webrev, so if you
>>> recreate the config files the diff to
>>> 'common/autoconf/generated-configure.sh' should be only the
>>> time-stamp field.
>>>
>>> Regards,
>>> Volker
>>>
>>> /Magnus
>>>
>>>
>>>
>>
More information about the ppc-aix-port-dev
mailing list