RFR (M): 8024265: Enable new build on AIX (top level part)
Vladimir Kozlov
vladimir.kozlov at oracle.com
Tue Sep 10 11:44:35 PDT 2013
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